[ZEBRA-1139] Merge OG sqlalchemy-continuum updates into our version #19
Conversation
little grammar fix
…ment improve memory usage for vacuum on large tables
Fixes for a couple of issues related to using multiple schemas
…y-continuum into oinopion-fix-changeset
As stated on kvesteri#101, quickstart example doesn't work as is. I've adapted the snippet to let it work without problem if someone copy and paste it.
As stated on kvesteri#101, quickstart example doesn't work as is. I've adapted the snippet to let it work without problem if someone copy and paste it.
Update snippets on documentation
…_relationships don't include excluded properties
Hotfix/track cloned connections (#2) Hotfix/track cloned connections (#3) change iterations use only ConnectionFairy as indexes create new dict entry for clones create entry using old connection create entry using session revert to event listening strategy remove cloned connections on rollback treat None case when cleaning connections Hotfix/track cloned connections (#2) Hotfix/track cloned connections (#3) change iterations use only ConnectionFairy as indexes create new dict entry for clones create entry using old connection create entry using session revert to event listening strategy remove cloned connections on rollback treat None case when cleaning connections
…onnections track cloned connections
Raising a new KeyError means you lose the original stacktrace, which is going to be more useful since it'll contain more information about the relevant context for this error.
…ption Reraise original exception instead of a new one
…ails without TransactionChanges plugin" and fix that has Transaction.entity_names raise a NoChangesColumn instead of AttributeError.
…ansaction_changes.py and tests/inheritance/test_single_table_inheritance.py
…ject to scalar subquery Tested by running `DB=sqlite py.test tests`. With master: 653 passed, 99 skipped, 4909 warnings in 65.90s (0:01:05) With this change: 653 passed, 99 skipped, 4695 warnings in 64.93s (0:01:04)
fix for unit_of_work.py:263: SAWarning: implicitly coercing SELECT object to scalar subquery
Following Flask-Login documentation we should use current_user.get_id() instead of current_user.id. (closes kvesteri#149) https://flask-login.readthedocs.io/en/latest/#your-user-class Signed-off-by: Jiri Kuncar <jiri.kuncar@cern.ch>
fix issue 85 "TransactionBase.changed_entities fails without TransactionChanges plugin"
Pass sync_triggers kwargs through to create_triggers Allows use of native versioning without mod tracking
…__upgrade-sqlalchemy
There was a problem hiding this comment.
just want to callout that this is a pretty high risk merge, so want to make sure we understand the impact and have a good way to test it before it goes through.
just going to put a "request changes" comment in here to block merging until i've vetted it more even though i don't have any requests at this time :)
|
I'm also going to add @minznerjosh as a reviewer here since I think he is the most familiar with our usage of this fork. |
|
@minznerjosh looks like a couple years ago you updated our fork based on sqlalchemy-continuum 1.3.9, and then reverted some of the changes here d4a87ba I'm trying to understand the safest path forward. We could:
|
Not touching it would limit us to not being able to update to python 3.10 in the future. The reason we want to update SQLAlchemy is to unblock that. That is a decision we can make short term, but long term we would want to unblock this. The other two options make sense to me and I'd defer to @minznerjosh's knowledge. My preferred would be to get up to date with upstream if there aren't clear blockers (or just directly using the package again if the blocker that initially got us to fork is resolved). |
|
Given that we're 63 commits ahead and 104 commits behind upstream, we're pretty diverged at this point and also I've struggled to find reasonable documentation on what changes we added. |
|
Agree with @MitulP91's thinking here. I think what we added comes down to two things:
FWIW, I'm pretty sure my rollback was due to regressions that the spark backend test suite caught. If our backend tests pass with these changes I'd personally feel quite confident in deploying. So it'd be great if we could somehow test this change in the spark repo before merging/publishing here. |
|
Good info. We should be able to make a branch in spark that points continuum to this branch instead of to a pinned version. Agree that we'd want to see tests pass in spark before merging here. |
In order to update our
SQLAlchemypackage inspark,sqlalchemy-continuumneeds to be upgraded. This PR follows the steps in this Syncing A Fork doc to get the latest code fromsqlalchemy-continuuminto our version of it.