-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Bare sqlalchemy metadata #2355
Bare sqlalchemy metadata #2355
Conversation
c4d9e9c
to
85e2cf5
Compare
38e58d1
to
e47e534
Compare
avoid chaining classmethod and property decorators This is deprecated since python3.11 and won't be supported in 3.13 and later
17e58c2
to
24a7909
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tested that existing functionality works. this will be a great extension to our model thanks for adding!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discovered this potential issue to document while testing main
@@ -300,7 +382,6 @@ def migrate(cls, autogenerate: bool = False) -> bool | None: | |||
return True | |||
|
|||
@classmethod | |||
@property |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this might be a breaking change... my example app rx_shout was using select
as a class property on a model.
And now it throws stack at runtime
Traceback (most recent call last):
File "/Users/masenf/code/reflex-dev/reflex/reflex/state.py", line 1454, in _process_event
events = fn(**payload)
^^^^^^^^^^^^^
File "/Users/masenf/code/reflex-dev/repro-datetime-model/rx_shout/state.py", line 48, in on_load
self.load_entries()
File "/Users/masenf/code/reflex-dev/repro-datetime-model/rx_shout/state.py", line 45, in load_entries
self.entries = session.exec(Entry.select.order_by(Entry.ts.desc())).all()
^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'function' object has no attribute 'order_by'
Workaround is to call the function instead of treating it like a property: Entry.select().order_by(...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's a breaking change. However, I still think it was the right decision to drop those deprecated decorator chains.
#2340