feat: port to SQLAlchemy 1.4 / 2.0

[hadrien]

Description

• Pagination now allows 2.0 sqla queries.

Related PRs

Blocked by

• chore: fix tox configuration #27

[codecov]

Codecov Report

Merging #28 (fe3ebc6) into master (78308f4) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master       #28   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines          128       148   +20     
  Branches         1         2    +1     
=========================================
+ Hits           128       148   +20     

Flag	Coverage Δ
sqlalchemy1.3	94.59% <71.42%> (-5.41%)	⬇️
sqlalchemy1.4	100.00% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
fastapi_sqla/__init__.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 78308f4...fe3ebc6. Read the comment docs.

[hadrien]

If you have bandwidth @vicrep 🙏🏽 ?

[vicrep]

Could you elaborate on why we need to make the query_counter a dependency? Could we not make it a pure function which takes as arguments the query, which could be either a legacy or non-legacy query?

IMO requiring you make a dependency that returns a callable as a counter function feels a little indirected from a consumer's PoV / is not a very straightforward interface.

Could we make the pagination dependency smart enough to then determine if the query is a session.query or a DbQuery, and only call session.execute() on the latter?

Honestly, as I think about it more, could it be worth having two separate dependencies, one to paginate classic SQLA queries, and one to paginate when passed a 1.4 DbQuery? That way no breaking changes, and I think there's enough overlap that most of the code would be reused between both, but that way we're super explicit.

[hadrien]

Could you elaborate on why we need to make the query_counter a dependency? Could we not make it a pure function which takes as arguments the query, which could be either a legacy or non-legacy query?

Dependency is nice to do this:

def query_count_dep(q: str = Query(...), session:Session=Depends())-> QueryCount:
    def query_count(query:DbQuery)->int:
        query = select(func.count()).where(User.name.like(q)
         return session.execute(query).scalar()

IMO requiring you make a dependency that returns a callable as a counter function feels a little indirected from a consumer's PoV / is not a very straightforward interface.

I agree.

Could we make the pagination dependency smart enough to then determine if the query is a session.query or a DbQuery, and only call session.execute() on the latter?

yes, it is actually what that PR achieves. Out of the box, it just works with legacy and modern queries.

Honestly, as I think about it more, could it be worth having two separate dependencies, one to paginate classic SQLA queries, and one to paginate when passed a 1.4 DbQuery? That way no breaking changes, and I think there's enough overlap that most of the code would be reused between both, but that way we're super explicit.

I will keep the same query_count and remove the query count dependency thing..

That PR breaks the scoping rule. It does 2 things: porting to SQL 2.0 queries. Introducing a query count dependency. I'll lean and remove the breaking change.

[vicrep]

Oh I totally agree that the dependency a nice "advanced" usage option; I just would offer a simpler interface for people who want something a little more straightforward :)

Sounds good about scoping, I think we can probably find a sleek way to allow users to do both (simple/pure counter, and advanced dependency based counter).

[vicrep]

Something similar to how the python JSON module lets you override encoders could be nice -- when calling json.dumps, you can pass either: a "default" encoder which overrides just the default method of the default encoder (as a pure function), or a "cls" argument to override the entire encoder class.

[hadrien]

not anymore a breaking change FYI @vicrep

[vicrep]

🚢 🎉