Skip to content

Support PreferRepeatableRead on explicit transactions by retrying - #616

Merged
msullivan merged 2 commits into
masterfrom
prefer-rr-retry
Apr 29, 2025
Merged

Support PreferRepeatableRead on explicit transactions by retrying#616
msullivan merged 2 commits into
masterfrom
prefer-rr-retry

Conversation

@msullivan

@msullivan msullivan commented Apr 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

The method for detecting the need to retry is kind of hacky.

We cache using the filename and line number of the caller.
@msullivan
msullivan marked this pull request as ready for review April 25, 2025 04:14
scotttrinh
scotttrinh approved these changes Apr 25, 2025
View reviewed changes

@scotttrinh scotttrinh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Makes sense to me, and matches what Yury and I did in the JS (minus the caching). 👍
@1st1

1st1 commented Apr 28, 2025

Copy link
Copy Markdown
Member

I don't think we should implement the cache. Anyone who inherits from our client will have a broken caching. Like this is what Credal is doing with gel-js.
@msullivan

msullivan commented Apr 28, 2025

Copy link
Copy Markdown
Member Author

I don't think we should implement the cache. Anyone who inherits from our client will have a broken caching. Like this is what Credal is doing with gel-js.

I don't fully follow?

I did put an escape hatch in the cache for dealing with cases where the client always has an extra wrapper function, or similar. We can also tweak the cache behavior so that it doesn't always default to serializable forever once it has been needed once; we could flip the state back.
@1st1

1st1 commented Apr 29, 2025

Copy link
Copy Markdown
Member

I did put an escape hatch in the cache for dealing with cases where the client always has an extra wrapper function, or similar.

Yeah, I saw that and the explaining comment, I'm just really on the fence here thinking, hey -- it's way too much magic, and then I'm flipping back thinking -- hey, it can save a bunch of roundtrips and should be done. Anyway, let's go with the cache, let's do it. Approving the PR.
@fmoor fmoor mentioned this pull request Apr 29, 2025
Closed
@msullivan
msullivan merged commit e1e25c0 into master Apr 29, 2025
@msullivan
msullivan deleted the prefer-rr-retry branch April 29, 2025 18:21
msullivan added a commit that referenced this pull request Apr 29, 2025
@msullivan
gel-python 3.1.0
78960e8 
Changes
=======

* Enable reflecting of `ext` types.
  (by @vpetrovykh in 80a6b52)

* Enable multiple modules for SQLModel reflection.
  (by @vpetrovykh in 8a7fdb5)

* Update SQLModel generator.
  (by @vpetrovykh in 1d1c937)

* Fix an issue with generating SQLModel with array props.
  (by @vpetrovykh in 2acffba)

* Add encode and decode for array of array.
  (by @dnwpark in cc10463 for #594)
* Improve nested array test skipping
  (by @elprans in 4dd19cb)

* Fix server binary detection in WSL
  (by @elprans in 0d60662 for #601)

* Drop Python 3.8 support
  (by @elprans in 1209adc for #600)

* Fix reused SSLContext ALPN bug
  (by @fantix in 8874060 for #602)

* Pin Django to ~5.1
  (by @msullivan in dff941e for #610)

* Make IsolationLevel enum values match the server names
  (by @msullivan in 7d2a401 for #611)

* Make TransactionOptions default to optional
  (by @msullivan in 553e3ce for #612)

* Implement PreferRepeatableRead, apply TransactionOptions to config state
  (by @msullivan in 46dae11 for #609)

* Support PreferRepeatableRead on explicit transactions by retrying
  (by @msullivan in e1e25c0 for #616)

* Make EnumValue support comparison to strings
  (by @msullivan in bb93ed7 for #615)

* Update AI RAG response parsing.
  (by @dnwpark in 9060b04 for #618)
@msullivan msullivan mentioned this pull request Apr 29, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants

@msullivan @1st1 @scotttrinh