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.

Sign up for GitHub

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

Jump to bottom

Update to SQLA 1.4 #471

Closed
lukasjuhrich opened this issue May 20, 2021 · 3 comments · Fixed by #475
Closed

Update to SQLA 1.4 #471

lukasjuhrich opened this issue May 20, 2021 · 3 comments · Fixed by #475
Assignees
@lukasjuhrich

Comments

@lukasjuhrich
Copy link
Collaborator

This does not mean using v2.0-style query syntax, just upgrading and not breaking things.
@lukasjuhrich lukasjuhrich self-assigned this May 20, 2021
@lukasjuhrich
Copy link
Collaborator Author

One big problem: We compile in some locations at import time.
This won't work anymore with v4 because at compile time, various things will be checked already and that requires for the mappers to be fully initialized.

property.py
        property_query_stmt.compile(
traffic.py
        traffic_history_query().compile(

We need to find a way to describe the view / function definitions lazily, or to circumvent using compilation at all.

@lukasjuhrich
Copy link
Collaborator Author

Basically, that problem stems from the difference that visit_create_view expects a query and compiles it itself, while visit_create_function expects a finished string as the definition.

One way to achieve the latter would be to allow for Function.definition to be of type Query instead of only str, and then teaching visit_create_function to compile the query if it isn't a string already.

@lukasjuhrich
Copy link
Collaborator Author

Perhaps that kind of complexity is a good reason to defer this task to after #67 has been finished.

@lukasjuhrich lukasjuhrich removed their assignment May 20, 2021
@lukasjuhrich lukasjuhrich self-assigned this May 24, 2021
lukasjuhrich added a commit that referenced this issue May 25, 2021
@lukasjuhrich
Make Function accept Selectables as well
b836ca6 
Now the `Function` itself is responsible for any statement compilation
that needs to happen.  Because of this, compilation can be done
lazily (i.e. with the first `.definition` property access), and does
not happen at import time anymore.

Fixes a compatability problem with SQLA1.4.

Refs #471
lukasjuhrich added a commit that referenced this issue May 25, 2021
@lukasjuhrich
Pass selectables instead of queries to Function defs
752ca27 
Refs #471
lukasjuhrich added a commit that referenced this issue May 25, 2021
@lukasjuhrich
Manually attach the column to the table in View construction
ebdfcd1 
For some reason, in the sqlalchemy
commit:aceefb508ccd0911f52ff0e50324b3fefeaa3f16
in lib/sqlalchemy/sql/elements.py, the `ColumnClause._make_proxy`
method does not automatically attach the column anymore.

This is now done manually.

Refs #471
lukasjuhrich added a commit that referenced this issue May 25, 2021
@lukasjuhrich
Mark custom type decorators as cache_ok to suppress warning
dbcfda2 
Refs #471
lukasjuhrich added a commit that referenced this issue May 25, 2021
@lukasjuhrich
Use selected_columns instecd of .c
c60e371 
SA warns that `SelectBase.c` is deprecated[1].

Refs #471

[1] https://docs.sqlalchemy.org/en/14/changelog/changelog_14.html#change-eb18d201ebe7209056b7c9d54f986b9e
lukasjuhrich added a commit that referenced this issue May 25, 2021
@lukasjuhrich
Remove redundant transaction rollback
2580e06 
This silences a (justified) warning from SQLAlchemy.

Loosely Refs #471.
lukasjuhrich added a commit that referenced this issue May 25, 2021
@lukasjuhrich
Remove redundant transaction rollback
74fe109 
This silences a (justified) warning from SQLAlchemy.

Loosely Refs #471.
lukasjuhrich added a commit that referenced this issue May 25, 2021
@lukasjuhrich
Only apply transaction rollback when necessary
5d52fc3 
This silences a (justified) warning from SQLAlchemy.

Loosely Refs #471.
lukasjuhrich added a commit that referenced this issue May 26, 2021
@lukasjuhrich
Mark custom type decorators as cache_ok to suppress warning
f8fee39 
Refs #471
lukasjuhrich added a commit that referenced this issue May 26, 2021
@lukasjuhrich
Use selected_columns instecd of .c
a4984f0 
SA warns that `SelectBase.c` is deprecated[1].

Refs #471

[1] https://docs.sqlalchemy.org/en/14/changelog/changelog_14.html#change-eb18d201ebe7209056b7c9d54f986b9e
lukasjuhrich added a commit that referenced this issue May 26, 2021
@lukasjuhrich
Only apply transaction rollback when necessary
bb761be 
This silences a (justified) warning from SQLAlchemy.

Loosely Refs #471.
lukasjuhrich added a commit that referenced this issue May 26, 2021
@lukasjuhrich
Make some convenience relationships viewonly=True
82fee52 
These are transitive counterparts to more complicated relationships.
For instance, `TaskLogEntry.user` is just a shortcut for the (partial)
relation TaskLogEntry → (Task as UserTask) → User.

Obviously this overlaps with the actual foreign key relations, so it
triggers a warning.

Clearly it should be marked `viewonly=True`, as these relationships
exist only for convenience.

Refs #471
lukasjuhrich added a commit that referenced this issue May 26, 2021
@lukasjuhrich
Fix with_polymorphic calls
2294421 
Refs #471
lukasjuhrich added a commit that referenced this issue May 26, 2021
@lukasjuhrich
Extract deprecated query into known_properties
44fccb3 
The motivation is that `.values()` is deprecated.  Instead, a
`future.select` has been used to ease transition to SQLAlchemy 2.0.

The query has been moved to the `lib` because there it is testable
more easily.

Refs #471
lukasjuhrich added a commit that referenced this issue May 26, 2021
@lukasjuhrich
Replace as_scalar by scalar_subquery
be91fe9 
Refs #471
lukasjuhrich added a commit that referenced this issue May 26, 2021
@lukasjuhrich
Extract users_eligible_for_fee_query
c4f1594 
This is necessary in order to compile and analyze the query without
copy'n'pasting large chunks in consoles.

Loosely refs #471, as SQLA now gives us tons of linting warnings
regarding accidental cartesian products (see [1]).

[1] https://docs.sqlalchemy.org/en/14/changelog/migration_14.html#built-in-from-linting-will-warn-for-any-potential-cartesian-products-in-a-select-statement
lukasjuhrich added a commit that referenced this issue May 26, 2021
@lukasjuhrich
Improve query filtering in users_eligible_for_fee_query
db5df99 
Instead of joining arbitrary properties with respect to two different
timestamps and then filtering, we now include the relevant filters
directly in the join, and afterwards only check if one of both is not
null.

Refs #471 because this change silences 6 warnings of the sqla query
linter.
@lukasjuhrich lukasjuhrich mentioned this issue May 26, 2021
Merged
2 tasks
lukasjuhrich added a commit that referenced this issue May 26, 2021
@lukasjuhrich
Change from old-style to 2.0-style select
e536892 
This means not passing a list, but plain `*args` instead.
easier to read as well :-)

Refs #471
lukasjuhrich added a commit that referenced this issue May 27, 2021
@lukasjuhrich
Use sqlalchemy.text instead of passing string to connection.execute
ef9f240 
See the example in
https://docs.sqlalchemy.org/en/14/changelog/migration_20.html#migration-to-2-0-step-two-turn-on-removedin20warnings

Refs #471
lukasjuhrich added a commit that referenced this issue May 27, 2021
@lukasjuhrich
Use a test engine with future=True
45567ab 
This allows for things like calling `connection.commit()`, see the
example in the docs[1].

[1] https://docs.sqlalchemy.org/en/14/changelog/migration_20.html#migration-core-connection-transaction

Refs #471
lukasjuhrich added a commit that referenced this issue May 27, 2021
@lukasjuhrich
Add assertion that a test object is not added session
c908177 
This helped me with figuring out whether the `cascade_backrefs=False`
parameter on the backref did the correct thing, and may as well stay
in the test.

Refs #471
lukasjuhrich added a commit that referenced this issue May 27, 2021
@lukasjuhrich
Set cascade_backrefs=False or viewonly on more relationships
5aa8153 
Again, see https://docs.sqlalchemy.org/en/14/changelog/migration_14.html#change-5150.

Relationships which require persistence operations as communicated by
explicit `cascade` settings have been marked with
`cascade_backrefs=False`, while others have been turned into
`viewonly` relationships, because they usually just exist to provide
read access; adding a one-to-many relationship is usually done the
other way (e.g. `foo.parent = bar` instead of
`bar.children.append(foo)`).

Refs #471
lukasjuhrich added a commit that referenced this issue May 27, 2021
@lukasjuhrich
Replace old select / insert calls by new ones
e4a59bb 
- select now uses `*args` instead of a list
- `join` does not support doing multiple joins at once by passing
  2-tuples, instead, we have to chain `.join()`
- `update`/`insert` don't accept `values=` anymore, instead we have to
  use the fluent `.values()` method

Refs #471
lukasjuhrich added a commit that referenced this issue May 27, 2021
@lukasjuhrich
Fix Property test
ef721a2 
there may or may not be other memberships floating around depending on
what the factories provide.

This way is also more direct and clear.

Refs #471
lukasjuhrich added a commit that referenced this issue May 27, 2021
@lukasjuhrich
Use text for session.execute invocations
843ac3f 
Refs #471
lukasjuhrich added a commit that referenced this issue May 27, 2021
@lukasjuhrich
Explicitly add something to session
488cb69 
This is necessary because `cascade_backrefs=False` disables the
implicit mechanism of attaching objects indirectly to our `session`.

Doing things like these explicitly is also beneficial because we can
now judge what goes into the session without having to be familiar
with implementation detais of all relevant `relationship`s

Refs #471
lukasjuhrich added a commit that referenced this issue May 27, 2021
@lukasjuhrich
Replace last subtransactions=True with begin_nested
aa70bfb 
Refs #471
lukasjuhrich added a commit that referenced this issue May 27, 2021
@lukasjuhrich
Fix dict conversion of Row
0999e1c 
See
https://docs.sqlalchemy.org/en/14/changelog/migration_20.html#result-rows-act-like-named-tuples

Refs #471
lukasjuhrich added a commit that referenced this issue May 27, 2021
@lukasjuhrich
Replace query.get by session.get
d192b3c 
Refs #471
lukasjuhrich added a commit that referenced this issue May 27, 2021
@lukasjuhrich
Fix various tests as a consequence of SQLA2.0 changes
afc9ff6 
For various reasons, they did not work with a `future=True` engine
and/or the adaptions of the recent commits.

- `flask_testing.TestCase.client` is mysteriously unavailable on the
  test class, causing the `tearDown` to fail (but, interestingly, not
  the setUp!).  My most recent guess wuold be that it is somehow
  related to the fact that `pytest-flask` provides a fixture named
  `client`.
  The workaround is to store `self.client` as `self.client2`
  (“Drehstulinterface”, as a wise German software developer once said)

- The `follow_redirects` flag was removed in a debugging attempt, and
  may as well stay absent as it serves no purpose.

- I have no idea how the BuildingFactory could ever have worked, as
  the `number` field always requires a `str` instead of an `int`.

Refs #471
lukasjuhrich added a commit that referenced this issue May 27, 2021
@lukasjuhrich
Don't remove the session after each app request when testing
c361202 
Refs #471

Also popped up in context of the recent SQLA2.0 changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lukasjuhrich lukasjuhrich

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

SQLAlchemy 1.4 / 2.0 compat agdsn/pycroft
1 participant
@lukasjuhrich