kvesteri / sqlalchemy-continuum

Versioning extension for SQLAlchemy.
BSD 3-Clause "New" or "Revised" License
578 stars 127 forks source link

SqlAlchemy 2.0 support #330

Closed alliefitter closed 1 year ago

alliefitter commented 1 year ago

This is a just a stopgap, but I've attempted to add support for SqlAlchemy 2.0. Most select() statements were able to be updated to 2.0 syntax without issue, while some would only work with orm.Query(). text() was added to any query literals. It seems that basic functionality is g2g, but there's some weirdness with the unit tests. The vast majority of top level tests succeed when running the entire suite, but many of the nested tests (relationships, plugins) failed unless they're run on their own. There are also some tests (test_savepoints.py) that either I'm not understanding the intent of or are just broken. Also, I skipped the flask tests, as I'm unfamiliar with the _SessionSignalEvents that's being imported in the test file, but it seems to have been removed from Flask-SQLAlchemy. I'm not sure how to adapt the tests to the current state of the flask plugin. I'm not sure what the state of tests were before this fork, and how many were regularly failing, so any context there would be greatly appreciated. I should note that I've only run the tests with sqlite, so the issues I'm running into may be specific that database.

alliefitter commented 1 year ago

Spent some more time on tests, and found a couple more bugs in the package. All tests, aside from i18n and test_savepoints are working with sqlite. Removing the mocking of _SessionSignalEvents's register method seems to have resolved any problems in plugins.test_flask, and there doesn't seem be any side effects from not mocking event listener registration in other tests, as the comment suggests. i18n is failing seemingly because it doesn't appear to be compatible with 2.0. The GitHub hasn't been touched since 2021, and 2.0 was released this year.

There are several test cases which hang when using postgres as the driver. The most recent commit has added some skipif for these tests. The intention there is to skip those tests temporarily in order to find errors specific to postgres, if any. I'm not sure what exactly is causing it, but when it happens, these queries are stuck in pg_stat_activity. The former's state is idle in transaction while the latter's is active. This is probably an obvious problem to someone more experienced with sqlalchemy than I am.

DROP SEQUENCE transaction_id_seq;
CREATE TABLE text_item (
    id SERIAL NOT NULL, 
    PRIMARY KEY (id)
);
marksteward commented 1 year ago

Thanks for this, will have an in-depth look at the weekend!

POD666 commented 1 year ago

Looks like it's left to upgrade flask-sqlalchemy? https://github.com/kvesteri/sqlalchemy-continuum/actions/runs/4704167948/jobs/8345165771?pr=330#step:6:80

alliefitter commented 1 year ago

Very odd, I just re-ran the flask tests locally, and they passed. Also, when I run the entire suite with sqlite configured, only test_savepoints fails.

Edit: Ah, I think it's because I've got Flask-SQLAlchemy 3.0.3 installed locally, and the setup.py is requiring >=1.0,<3.0.0

POD666 commented 1 year ago

Any updates here?

marksteward commented 1 year ago

I'm tracking this under #326, and just last night fixed the issue that was causing problems. I'll be cutting a 2.0 release shortly.

progmancod commented 1 year ago

Any news?

marksteward commented 1 year ago

I've just released 1.4, which should make 2.0 usable. Let me know how you get on!

marksteward commented 1 year ago

Fixed by:

5a5ae91 0cba780 214cd5a 02cd7aa b907593 33af1b5 45733b0

etc

marksteward commented 1 year ago

Thanks for your help getting this started, @alliefitter!

alliefitter commented 1 year ago

No worries @marksteward! Thanks for getting working correctly, lol.