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

Debug SQLAlchemy 2.0 .scalars().all() use #1473

Open
xlorepdarkhelm opened this issue Dec 10, 2024 · 1 comment
Open

Debug SQLAlchemy 2.0 .scalars().all() use #1473

xlorepdarkhelm opened this issue Dec 10, 2024 · 1 comment
Assignees
Labels
bug engineering python Pull requests that update Python code

Comments

@xlorepdarkhelm
Copy link
Contributor

xlorepdarkhelm commented Dec 10, 2024

Fixing the db.session.execute(stmt).scalars().all() problem.

This was part of the move to SQLAlchemy 2.0, and it could be very problematic. scalars() gets a single value from a row, while all() gets all the rows and puts them into a list. So this makes a list of single values. Which is fine, if and only if we only want a single value from the row. But if you want all the columns that are loaded (like from a invited user model), this breaks a lot of things, and has unknown results.

We need to go through all uses of this pattern in the API code and verify - are we wanting just a single column's data to be returned, or multiple columns of data returned. If multiple columns of data, then we need to remove the .scalars() part of that code.

We might need to verify all other uses of .scalars() or .scalar() beyond those that use .all() too.

@xlorepdarkhelm xlorepdarkhelm converted this from a draft issue Dec 10, 2024
@xlorepdarkhelm xlorepdarkhelm added engineering bug python Pull requests that update Python code labels Dec 10, 2024
@xlorepdarkhelm xlorepdarkhelm self-assigned this Dec 10, 2024
@xlorepdarkhelm xlorepdarkhelm moved this from 🌱 New to 🏗 In progress (WIP: ≤ 3 per person) in Notify.gov product board Dec 10, 2024
@ccostino
Copy link
Contributor

Noting here that we still have an open PR for SQLAlchemy updates: #1417

We should go through this more closely given this insight and make adjustments before merging it in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug engineering python Pull requests that update Python code
Projects
Status: 🏗 In progress (WIP: ≤ 3 per person)
Development

No branches or pull requests

2 participants