Skip to content
This repository was archived by the owner on Aug 19, 2025. It is now read-only.

🪛 Moving to SQLAlchemy 2.0#540

Merged
tarsil merged 10 commits into
encode:masterfrom
tarsil:feature/add_sqlalchemy_2
Feb 21, 2024
Merged

🪛 Moving to SQLAlchemy 2.0#540
tarsil merged 10 commits into
encode:masterfrom
tarsil:feature/add_sqlalchemy_2

Conversation

@tarsil

@tarsil tarsil commented Mar 22, 2023

Copy link
Copy Markdown
Contributor
  • Added support for python 3.11
  • Added common and dialects packages to handle the new SQLAlchemy 2.0+

@aminalaee @Kludex this is a cleaner version of the previous PR with the changes passing the new SQLAlchemy 2.0 which is significantly different from the < 2.0

It is also good to mention that when I was doing this PR, I was reading some suggestions and the one from here #507 was good to add it as well. So I would like to thank @circlingthesun and @JGoutin for the suggestions

Edit: The asyncpg sometimes hangs during the tests. I forcibly close the connections during that test to make sure it doesn't get stuck

tarsil added 2 commits March 22, 2023 16:15
* Removed support for python 3.7
* Added common and dialects packages to handle
the new SQLAlchemy 2.0+
* The connections were hanging sometimes
during the tests.
@tarsil

tarsil commented Mar 22, 2023

Copy link
Copy Markdown
Contributor Author

@aminalaee @Kludex maybe you could manually kill:

Please? This was the whole reason why the second commit. Quite a well known issue but that CI passed with flying colours, only these 2 are hanging

@tarsil tarsil changed the title 🪛 Added support for SQLAlchemy 2.0 🪛 Moving to SQLAlchemy 2.0 Mar 22, 2023
@tarsil tarsil mentioned this pull request Mar 22, 2023
@Kludex

Kludex commented Mar 22, 2023

Copy link
Copy Markdown
Contributor

Done

@tarsil

tarsil commented Mar 22, 2023

Copy link
Copy Markdown
Contributor Author

Well, I hope this is ok. Quite nice and clean integration for SQLAlchemy 2.0

@lucasguiss

Copy link
Copy Markdown

I've forked the version of this PR and it's working fine in our apps, waiting for the release 😄

@tarsil

tarsil commented Mar 22, 2023

Copy link
Copy Markdown
Contributor Author

@lucasguiss very happy to hear that :)

@tarsil

tarsil commented Mar 22, 2023

Copy link
Copy Markdown
Contributor Author

@Kludex here the status is not "complete" because this PR also makes sure python 3.7 is no longer supported for many reasons but also in June the support will stop.

@Kludex

Kludex commented Mar 22, 2023

Copy link
Copy Markdown
Contributor

@Kludex here the status is not "complete" because this PR also makes sure python 3.7 is no longer supported for many reasons but also in June the support will stop.

Why did you remove now?

@tarsil

tarsil commented Mar 22, 2023

Copy link
Copy Markdown
Contributor Author

@Kludex here the status is not "complete" because this PR also makes sure python 3.7 is no longer supported for many reasons but also in June the support will stop.

Why did you remove now?

Well, I can always add and we remove after june, I would say. The initial tests were failing with some 3.7 issues but I will add it back. I think I solved them

@Kludex

Kludex commented Mar 22, 2023

Copy link
Copy Markdown
Contributor

Can we remove all changes that are not related to the support of SQLAlchemy 2.0? You can create other PRs for them, but let's focus on a single thing here, please.

@tarsil

tarsil commented Mar 22, 2023

Copy link
Copy Markdown
Contributor Author

Can we remove all changes that are not related to the support of SQLAlchemy 2.0? You can create other PRs for them, but let's focus on a single thing here, please.

@Kludex Done. It was mainly that, had initial configuration issues with python 3.7 but that was addressed.

The changes you see are the ones used for the move to SQLAlchemy 2.0+

Regarding other PRs. I can do that later once the time comes. This was already a good change that allow us to move forward.

@Kludex Kludex requested a review from aminalaee March 22, 2023 22:16
Comment thread CHANGELOG.md
Comment thread .github/workflows/test-suite.yml
Comment thread .github/workflows/publish.yml
Comment thread README.md
Comment thread README.md Outdated
Comment thread mkdocs.yml
Comment thread docs/index.md
Comment thread docs/index.md Outdated
tarsil added 2 commits March 24, 2023 10:26
* Revert changes automatically applied by IDE
* Fix formatting of CI caused by IDE changes
* Codebase cleanup for SQLAlchemy 2.0
* Fix EOL in mkdocs
@tarsil tarsil force-pushed the feature/add_sqlalchemy_2 branch 2 times, most recently from 23223c9 to b271a54 Compare March 24, 2023 10:31
@tarsil tarsil force-pushed the feature/add_sqlalchemy_2 branch from b271a54 to deedd13 Compare March 24, 2023 10:32
@tarsil

tarsil commented Mar 26, 2023

Copy link
Copy Markdown
Contributor Author

Hey @aminalaee . Any news about this? I know you are extremely busy

@aminalaee

Copy link
Copy Markdown
Contributor

Hi @tarsil , I will take a look today or tomorrow. Sorry for the delay.

@mathause

Copy link
Copy Markdown

I think you'll have to update setup.py as well:

install_requires=["sqlalchemy>=1.4.42,<1.5"],

@tarsil

tarsil commented Mar 26, 2023

Copy link
Copy Markdown
Contributor Author

I think you'll have to update setup.py as well:

install_requires=["sqlalchemy>=1.4.42,<1.5"],

I did update it as well before. Its there 👍🏼

@mathause

Copy link
Copy Markdown

pip complained when I installed the branch, but I see it now. Not sure why I missed it before - sorry for the noise.

This was referenced Mar 27, 2023
@tarsil

tarsil commented Mar 30, 2023

Copy link
Copy Markdown
Contributor Author

@aminalaee sorry to a bother but, any news?

@tarsil

tarsil commented Apr 27, 2023

Copy link
Copy Markdown
Contributor Author

@Kludex this ia probably my fault. Since our last conversation unfortunately I didn't have enough time to address these comments. @rafalp in your code comment. I know SQLA stopped supporting that and that is the reason why I didn't change too much and I simple added the * as per normal python.

I promise I will try to address all the comments soon. Apologies

@MAKMED1337

MAKMED1337 commented Apr 30, 2023

Copy link
Copy Markdown

Few days ago SQLAlchemy released 2.0.11 where they have changed Row class
Will it be supported ?

return raw

def __iter__(self) -> typing.Iterator:
return iter(self._row.keys())

@MAKMED1337 MAKMED1337 Apr 30, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Because of this you can't unpack result anymore
Example:

class LastUsername(Base):
	__tablename__ = 'last_username'
	user_id = Column(BIGINT, primary_key=True)
	username = Column(TEXT, nullable=False)

r = await db.fetch_one(select(LastUsername))
user_id, username = r
assert user_id == 'user_id' #but it need to be at least int

@stonebig

stonebig commented Jun 4, 2023

Copy link
Copy Markdown

hum, any hope for this to be merged ?

@Kludex

Kludex commented Jun 4, 2023

Copy link
Copy Markdown
Contributor

hum, any hope for this to be merged ?

If comments are addressed or answered, we can continue here

@Feijo

Feijo commented Jun 4, 2023

Copy link
Copy Markdown

Honestly I just decided to give up on this library altogether and use the new async features built-in into SA2.

@stlomib

stlomib commented Jul 6, 2023

Copy link
Copy Markdown

Any update?

1 similar comment
@poliroid

poliroid commented Aug 8, 2023

Copy link
Copy Markdown

Any update?

@tarsil

tarsil commented Aug 8, 2023

Copy link
Copy Markdown
Contributor Author

Sorry, didn't have enough time to fully address this but I will soon

@philipbjorge

Copy link
Copy Markdown

@tarsil --
Thanks for putting this together --
We've put this in one of our projects and our test suite was 🟢.

We'll be kicking the tires more on this over the coming weeks and sharing any feedback or discoveries here.

(We did pin on SA 2.0.10 based on this comment)

@ziglef

ziglef commented Nov 28, 2023

Copy link
Copy Markdown

Any updates or roadmap for this PR?

@philipbjorge

Copy link
Copy Markdown

We just shipped this branch out to production and haven't noticed any problems yet.
We're a fairly low volume service, but overall feeling very good about this branch.
We're using this as a bridge while we port over to SA2.

@adnam

adnam commented Dec 5, 2023

Copy link
Copy Markdown

I've tried out this branch by specifying the following in my requirements file:

databases @ git+https://github.com/tarsil/databases@feature/add_sqlalchemy_2

which resulted in pip-compile choosing sqlalchemy==2.0.23 - the latest version as of Nov 2 '23.

As previously mentioned here the Row class has changed in v2.0.11 resulting in an error like this when performing a query:

AttributeError: type object 'Row' has no attribute '_default_key_style'

I think it would be worth updating this PR to account for the new Row class before merging, otherwise projects using databases will be pinned to an sqlalchemy version from April '23.

@tarsil I'm not very familiar with SqlAlchemy internals but the solution would be something like this: adnam@96e15fd

@pmdevita

Copy link
Copy Markdown
Contributor

@ansipunk solved this here tarsil#2 Just need to get this merged and we'll be back to 100% tests passing.
What else needs to be done for this PR to be merged?

Fix compatibility with SQLAlchemy>=2.0.11
@tarsil

tarsil commented Feb 20, 2024

Copy link
Copy Markdown
Contributor Author

@ansipunk solved this here tarsil#2 Just need to get this merged and we'll be back to 100% tests passing. What else needs to be done for this PR to be merged?

It is merged and I do apologize for not receiving notifications and missing this.

@Kludex

Kludex commented Feb 21, 2024

Copy link
Copy Markdown
Contributor

When @tarsil is happy (and this is ready to be merged...), I'll merge it and make a release. Ping me.

@tarsil

tarsil commented Feb 21, 2024

Copy link
Copy Markdown
Contributor Author

@Kludex i do believe I just need to fix the conflicts and we should be good to go

@Kludex

Kludex commented Feb 21, 2024

Copy link
Copy Markdown
Contributor

I've invited you to encode @tarsil . Ask me on PM if you have questions about workflows or anything.

@Kludex Kludex 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.

Merge when happy @tarsil 👍

@tarsil tarsil merged commit 1e40ad1 into encode:master Feb 21, 2024
@tarsil tarsil deleted the feature/add_sqlalchemy_2 branch February 21, 2024 12:26
@tarsil

tarsil commented Feb 21, 2024

Copy link
Copy Markdown
Contributor Author

@Kludex , I just merged after fixing the minor conflicts (normal as it was an old branch) and passed 👍🏼

@tarsil

tarsil commented Feb 21, 2024

Copy link
Copy Markdown
Contributor Author

I've invited you to encode @tarsil . Ask me on PM if you have questions about workflows or anything.

Will do but so far was just this simple merge conflict, I believe. Thank you for the invite.

@ghost ghost mentioned this pull request Feb 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.