build(deps): switch to sqlalchemy 1.4 by PrettyWood · Pull Request #299 · encode/databases

PrettyWood · 2021-03-15T21:34:07Z

Sqlalchemy 1.4 is out with some breaking changes (e.g. RowProxy got removed in favor of Row or the removal of row.values() in favor of row._mapping.values())

closes #298 and closes #348

PrettyWood · 2021-03-16T11:14:57Z

databases/backends/postgres.py

-        return self._row.values()
+    @property
+    def _mapping(self) -> asyncpg.Record:
+        return self._row


I would like your insight on this matter.

For postgres, row.values() was still working thanks to this Record class but for all other backends we now need to do row._mapping.values() with sqlalchemy >= 1.4

Do we want to:

keep values() only for postgres

keep values() for all backends and we can attach the deprecated property to the sqlalchemy.Row class

let it like this and mimic sqlalchemy API also for postgres

Thanks for this very nice lib!

imo It'd be better to keep row.values() for all backends since looking at test changes I can see that row.keys() is still present so row._mapping.values() would bring an inconsistency.

Although it's sorta weird they kept row.keys() but got rid of row.values() in favor of row._mapping.values(). I think you could try row._asdict() as suggested here in SQLAlchemy issue related to 1.4.

SQLAlchemy documentation says row.keys() is deprecated in favor of row._mapping() so I suppose you have to add row.mapping() for all backends.

Hmmm I don't really want to add row.values() for all backends since it would need some monkeypatching or wrapper around the SQLAlchemy objects.
I reckon the idea of databases is to keep things simple and to stay close to SQLAlchemy when possible.
This is why I went with updating postgres backend only to mimic SQLAlchemy behaviour.
But honestly I don't have a strong opinion on this so if people really want values(), keys() back, we can recreate the wrapper RowProxy

My bad, I started my comment with keep since that inconsistency was triggering me, although after reading SQLAlchemy's changelogs and docs I immediately changed my mind about keeping values but did not change the original paragraph.

I do agree with decision to keep the API as close to SQLAlchemy's as possible.

Just that test having both row.values and row._mapping.keys misled me.

uSpike · 2021-03-26T14:17:18Z

setup.py needs to be updated to specify install_requires=["sqlalchemy>=1.4,<2", ... or a similar constraint.

lovelydinosaur · 2021-03-26T15:21:30Z

New release 0.4.3 to properly pin SQLAlchemy until this PR is resolved. https://pypi.org/project/databases/

(This PR should update the pin to <1.5, so we don't get bitten again.)

PrettyWood · 2021-04-03T22:30:44Z

@tomchristie do we want to support both 1.3 and 1.4 or 1.4 only?

lovelydinosaur · 2021-04-04T09:19:25Z

Ideally both, at least for a time. (But if that was really awkward I guess we could consider just 1.4, with some clear notes for users on which older version of databases to use for 1.3 etc)

PrettyWood · 2021-04-06T09:32:11Z

@tomchristie Ok no problem. Is it ok if I update CI pipeline to run all tests with both versions?

ansipunk · 2021-05-05T09:41:32Z

I guess dropping support for 1.3 is okay since pip 's new dependency resolver would automatically download an older version of databases if the user's environment requires SQLAlchemy 1.3.

An ugly solution to support both 1.3 and 1.4 can be seen here in SQLAlchemy-Utils 0.37.

lovelydinosaur · 2021-05-05T09:59:43Z

@Euromance Fair point. I'm okay with requiring 1.4+ then.

PrettyWood · 2021-05-07T07:00:19Z

Cool! I'll get back on it next Thursday when I have more time

ansipunk

I guess you could also test that deprecated methods throw their warnings

PrettyWood · 2021-05-13T14:28:48Z

@Euromance Yes it was planned thanks 👍 I first wanted to have your feedback on the interface. Glad we agree! Coverage should be good now

nikolayanguelov · 2021-06-29T09:29:57Z

Do you guys have any update on the ETA on when this might get merged and released to the public?

Thank you!

pvicente · 2021-07-22T08:29:44Z

Hi, is there any plan to merge this PR? When are you planning to do a release with sqlalchemy >= 1.4 support? 🙏🏼

lovelydinosaur · 2021-08-16T10:59:59Z

@PrettyWood If you'd like to get this in then let me know and can add you to the maintainers team, and walk you through the release process.

PrettyWood · 2021-08-16T20:18:57Z

With pleasure @tomchristie!

lovelydinosaur · 2021-08-17T11:19:42Z

@PrettyWood - Okay, I've sent you an invite. I'll leave getting this reviewed/merged/conflict resolved/whatever up to you - tho let us know if you need anything. Then once that's done let's walk through how you can roll a release.

PrettyWood · 2021-08-17T15:42:29Z

@tomchristie I rebased on master. I'll wait for a review from a member. If everything is fine, I feel like the release process is straightforward (PR to bump version with changelog update + tag that triggers github worfklow to publish on pypi). Am I missing something?

setup.py

ghost · 2021-08-24T21:39:04Z

Is this ready to be merged? Thank you!

Feijo · 2021-08-24T21:41:10Z

I am also waiting for this release

aminalaee · 2021-08-25T15:28:14Z

@PrettyWood Do you want to make any other changes or are you waiting for more reviewers?

PrettyWood · 2021-08-25T21:02:51Z

Hi @aminalaee
I don't plan to add anything more. If we feel like 1 review is enough then we can merge :)

aminalaee · 2021-08-26T05:47:49Z

Hi, @PrettyWood
I did go over this again and everything looks good. I think we can merge this and prepare the release #370

PrettyWood force-pushed the fix/sqlalchemy-1.4 branch 2 times, most recently from 89ab2b9 to 8aa58f3 Compare March 16, 2021 10:31

PrettyWood changed the title ~~fix: support sqlalchemy 1.4~~ build(deps): switch to sqlalchemy 1.4 Mar 16, 2021

This was referenced Mar 16, 2021

Allow collections.abc.Callable to be used as type in python 3.9 pydantic/pydantic#2519

Merged

fix(encoder): serialize NameEmail to str pydantic/pydantic#2479

Merged

PrettyWood force-pushed the fix/sqlalchemy-1.4 branch from 8aa58f3 to 36df25a Compare March 16, 2021 11:10

PrettyWood commented Mar 16, 2021

View reviewed changes

This was referenced Mar 22, 2021

Improve generic subclass support pydantic/pydantic#2549

Merged

added KafkaDsn to networks.py and add default ports for HttpUrl pydantic/pydantic#2447

Merged

uSpike mentioned this pull request Mar 26, 2021

Pin sqlalchemy to versions before 1.4 Kludex/starlette#1156

Closed

Kludex mentioned this pull request Mar 26, 2021

Please support latest SQLAlchemy or pin it fastapi/fastapi#2996

Closed

Kludex mentioned this pull request Mar 27, 2021

📌 Pin SQLAlchemy range for tests, as it doesn't use SemVer fastapi/fastapi#3001

Merged

collerek mentioned this pull request Apr 6, 2021

Update sqlalchemy to support >=1.4 collerek/ormar#142

Closed

lovelydinosaur mentioned this pull request Apr 21, 2021

Version 0.4.3 #314

Merged

ansipunk mentioned this pull request May 5, 2021

Broken RowProxy import with sqlalchemy 1.4 #298

Closed

PrettyWood force-pushed the fix/sqlalchemy-1.4 branch from 36df25a to 9d6e0c0 Compare May 13, 2021 09:28

ansipunk reviewed May 13, 2021

View reviewed changes

mweinelt mentioned this pull request Jun 18, 2021

when can we expect the support for SQLAlchemy >=1.4? #348

Closed

ToGoBananas approved these changes Aug 13, 2021

View reviewed changes

PrettyWood added 2 commits August 17, 2021 17:16

build(deps): switch to sqlalchemy 1.4

11aed30

fix deprecation warning and add tests

bce05db

PrettyWood force-pushed the fix/sqlalchemy-1.4 branch from 40c41c2 to bce05db Compare August 17, 2021 15:37

aminalaee reviewed Aug 17, 2021

View reviewed changes

setup.py Show resolved Hide resolved

aminalaee approved these changes Aug 17, 2021

View reviewed changes

aminalaee merged commit 89efe60 into encode:master Aug 26, 2021

PrettyWood deleted the fix/sqlalchemy-1.4 branch August 26, 2021 16:33

ugtar mentioned this pull request Oct 11, 2021

Postgres backend Record is a Mapping but some Mapping methods are deprecated #407

Closed

Conversation

PrettyWood commented Mar 15, 2021 • edited by aminalaee Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PrettyWood Mar 16, 2021

Choose a reason for hiding this comment

Uh oh!

ansipunk May 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PrettyWood May 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ansipunk May 13, 2021

Choose a reason for hiding this comment

Uh oh!

uSpike commented Mar 26, 2021

Uh oh!

lovelydinosaur commented Mar 26, 2021

Uh oh!

PrettyWood commented Apr 3, 2021

Uh oh!

lovelydinosaur commented Apr 4, 2021

Uh oh!

PrettyWood commented Apr 6, 2021

Uh oh!

ansipunk commented May 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lovelydinosaur commented May 5, 2021

Uh oh!

PrettyWood commented May 7, 2021

Uh oh!

ansipunk left a comment

Choose a reason for hiding this comment

Uh oh!

PrettyWood commented May 13, 2021

Uh oh!

nikolayanguelov commented Jun 29, 2021

Uh oh!

pvicente commented Jul 22, 2021

Uh oh!

lovelydinosaur commented Aug 16, 2021

Uh oh!

PrettyWood commented Aug 16, 2021

Uh oh!

lovelydinosaur commented Aug 17, 2021

Uh oh!

PrettyWood commented Aug 17, 2021

Uh oh!

Uh oh!

ghost commented Aug 24, 2021

Uh oh!

Feijo commented Aug 24, 2021

Uh oh!

aminalaee commented Aug 25, 2021

Uh oh!

PrettyWood commented Aug 25, 2021

Uh oh!

aminalaee commented Aug 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

PrettyWood commented Mar 15, 2021 •

edited by aminalaee

Loading

ansipunk May 6, 2021 •

edited

Loading

PrettyWood May 13, 2021 •

edited

Loading

ansipunk commented May 5, 2021 •

edited

Loading