Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrating SQLALchemy 2.0 #539

Closed
wants to merge 20 commits into from
Closed

Conversation

tarsil
Copy link
Contributor

@tarsil tarsil commented Mar 21, 2023

Attention

  • This introduces the minimum requirement of python 3.8 (3.7 EOL support is in June anyway)
  • Removed support for aiopg as it does not support SQLA 2.0 (so far). It can be added back later if they decide to add it.
  • Added fix for the _CompileLabel introduced as annotations for SQLA 2.0
  • Other minor fixes and updates in the docs
  • Removed tests that are no longer required.
  • Updated tests to remove references to aiopg

@Kludex and @aminalaee - I didn't have a lot of time recently and this, at least locally, was passing the current tests

@tarsil
Copy link
Contributor Author

tarsil commented Mar 21, 2023

Weird, locally it was passing but I was using Postgres only to be honest. Hmm, it would be great to have some suggestions. As I mentioned before, didn't have too much time on this, unfortunately.

databases/__init__.py Outdated Show resolved Hide resolved
tarsil and others added 3 commits March 22, 2023 02:08
Co-authored-by: Marcelo Trylesinski <marcelotryle@gmail.com>
* Moved common functions to a common package
* Created common Record for the DB supported
* Add docker-compose.yml for local development
* Added Makefile for local automations
Copy link
Member

@aminalaee aminalaee left a comment

Choose a reason for hiding this comment

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

Thank you for the effort, just a general note.


The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
Copy link
Member

Choose a reason for hiding this comment

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

I think the PR is getting way too big, for no reason. can we please stick only to necessary changes?
For example I think CHANGELOG.md is consistent withing all encode projects.

* Added new pg compiler
* Updated tests
@tarsil
Copy link
Contributor Author

tarsil commented Mar 22, 2023

@Kludex and @aminalaee I can put back the changelog to the way it was. I just thought it was to be consistent with Starlette and other products. Usually when people go after docs, having the release notes there helps. I'm just fixing some of the issues with the CI and this should be ok.

I was able to reintegrate with aiopg as well and the changes themselves not only cleans but also updates a lot of things based on SQLAlchemy 2.0

@tarsil
Copy link
Contributor Author

tarsil commented Mar 22, 2023

I will check what is happening here with the CI and then I will close this PR and open a clean new one @aminalaee and @Kludex

@tarsil tarsil closed this Mar 22, 2023
@tarsil tarsil reopened this Mar 22, 2023
@tarsil
Copy link
Contributor Author

tarsil commented Mar 22, 2023

Opening a brand new and cleaner PR

@tarsil tarsil closed this Mar 22, 2023
@tarsil tarsil deleted the feature/sqla_2 branch March 22, 2023 16:10
@zanieb zanieb mentioned this pull request Jul 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants