rusackas commented on code in PR #42787:
URL: https://github.com/apache/superset/pull/42787#discussion_r3722444754
##########
superset/databases/ssh_tunnel/models.py:
##########
@@ -55,6 +55,10 @@ class SSHTunnel(AuditMixinNullable, ExtraJSONMixin,
ImportExportMixin, Model):
uselist=False,
cascade="all, delete-orphan",
lazy="joined",
+ # SQLAlchemy 2.0 behavior: assigning `ssh_tunnel.database` no
+ # longer cascades the SSHTunnel into the Database's session;
+ # callers must add objects to a session explicitly.
Review Comment:
Already checked that part, ran the suite with `SQLALCHEMY_WARN_20=1` and the
new `pytest.ini` lines as hard errors, all 426 pass. So none of the three
actually lean on the old implicit cascade, they already either call
`session.add()` explicitly or go through the forward collection assignment,
which has its own real cascade untouched by this flag.
On the comments themselves, I'd lean toward keeping them just to match the
other seven models that got the identical treatment earlier in this series. Not
a hill I need to die on though, curious what you think. @hy144328 might have an
opinion too since he's driven most of this convention.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
