Skip to content

Dialect: Add methods concerned with isolation levels as no-ops#217

Merged
amotl merged 1 commit into
mainfrom
isolation-level-noop
Jun 4, 2026
Merged

Dialect: Add methods concerned with isolation levels as no-ops#217
amotl merged 1 commit into
mainfrom
isolation-level-noop

Conversation

@amotl
Copy link
Copy Markdown
Contributor

@amotl amotl commented Apr 2, 2025 •
edited
Loading

About

This was needed for unlocking mcp-alchemy, because it called set_isolation_level() on the dialect instance.

References

@amotl amotl mentioned this pull request Apr 2, 2025
Merged
amotl
amotl commented Apr 2, 2025
View reviewed changes
Comment thread src/sqlalchemy_cratedb/dialect.py Outdated
Comment on lines +209 to +216
def get_isolation_level_values(self, dbapi_conn):
return ()

def set_isolation_level(self, dbapi_connection, level):
pass

def get_isolation_level(self, dbapi_connection):
return "NONE"
Copy link
Copy Markdown
Contributor Author

@amotl amotl Apr 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any recommendations?

Copy link
Copy Markdown
Contributor Author

@amotl amotl Apr 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consulting Wikipedia and the SQLAlchemy documentation, READ UNCOMMITTED might be the right choice for CrateDB?

Copy link
Copy Markdown
Contributor Author

@amotl amotl Apr 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typical PostgreSQL drivers in Java (JDBC) and Python offer the same quartet, and Apache Spark additionally seems to provide NONE.

So, I guess using READ UNCOMMITTED as the default and single valid choice for CrateDB is the right way to resolve the situation?

Copy link
Copy Markdown
Contributor Author

@amotl amotl Apr 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like specifically for SQLAlchemy, AUTOCOMMIT seems to be more appropriate.

SQLAlchemy treats the concept of “autocommit” like any other isolation level; in that it is an isolation level that loses not only “read committed” but also loses atomicity. [...] This usually means that the typical DBAPI behavior of emitting BEGIN to the database automatically no longer occurs.

-- https://docs.sqlalchemy.org/en/20/core/connections.html#dbapi-autocommit

This was referenced Apr 2, 2025
Draft
Closed
@amotl amotl mentioned this pull request May 19, 2025
Open
@amotl amotl mentioned this pull request Nov 10, 2025
Closed
@amotl amotl force-pushed the isolation-level-noop branch from 1bad06d to b3896e3 Compare June 2, 2026 20:02
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 2, 2026 •
edited
Loading

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2d97300a-3922-490d-a64c-c5e8e4d2d3db

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch isolation-level-noop

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@amotl amotl requested a review from bgunebakan June 2, 2026 20:02
@amotl amotl force-pushed the isolation-level-noop branch from b3896e3 to fc5c65a Compare June 2, 2026 20:13
@amotl amotl marked this pull request as ready for review June 2, 2026 20:15
@amotl amotl linked an issue Jun 2, 2026 that may be closed by this pull request
Error: AttributeError: 'CrateDialect' object has no attribute 'default_isolation_level'. Did you mean: 'get_default_isolation_level'? #237
Closed
@amotl amotl requested a review from kneth June 3, 2026 10:02
@amotl amotl force-pushed the isolation-level-noop branch from fc5c65a to 9b19d3f Compare June 4, 2026 22:16
@amotl amotl mentioned this pull request Jun 4, 2026
Open
@amotl amotl merged commit a3be051 into main Jun 4, 2026
38 checks passed
@amotl amotl deleted the isolation-level-noop branch June 4, 2026 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kneth kneth kneth approved these changes

@bgunebakan bgunebakan Awaiting requested review from bgunebakan

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Error: AttributeError: 'CrateDialect' object has no attribute 'default_isolation_level'. Did you mean: 'get_default_isolation_level'?

2 participants

@amotl @kneth