rusackas commented on PR #42365:
URL: https://github.com/apache/superset/pull/42365#issuecomment-5064718544

   Capturing the follow-up work from the side discussion, since I'd rather keep 
this PR mechanical and easy to review than grow it.
   
   Adopting `with session.begin():` as the house style for new migrations seems 
like the obvious first step, basically what you already did in #42277.
   
   Converting the back catalog is optional and separable: the trailing-commit 
and try/commit/except/rollback shapes convert cleanly, but the loop/batch 
commits change failure semantics, so I'd leave those alone unless there's a 
concrete reason to touch one.
   
   Then there's deciding whether to lint migrations for real instead of 
excluding them. That would need a migration-appropriate message (there's no 
`@transaction` decorator in alembic-land), a consistent trigger (migration-only 
PRs currently skip the hook entirely), and a plan for `paginated_update`, which 
commits mid-generator.
   
   Happy to open issues for these if there's appetite. Thanks again for the PR!


-- 
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]

Reply via email to