Fix log template sync failing when a custom SQL Alchemy schema is configured - #70793
Open
Aaryan123456679 wants to merge 3 commits into
Open
Fix log template sync failing when a custom SQL Alchemy schema is configured#70793Aaryan123456679 wants to merge 3 commits into
Aaryan123456679 wants to merge 3 commits into
Conversation
…figured When sql_alchemy_schema is set, reflect_tables() derived the schema to reflect a mapped class's table under by splitting its bare __tablename__ on a dot, which discards the schema entirely for classes like LogTemplate whose table name has no dot. synchronize_log_template() then looked up the reflected table by that same bare name, which also does not match how SQLAlchemy keys schema-qualified tables in the reflected metadata. Together these left the log_template table unrecognized under a custom schema, causing triggering a Dag to fail with a TypeError. Derive the schema from the mapped class's own configured table instead of parsing its name, and look the reflected table up by its schema -qualified key.
eladkal
reviewed
Jul 31, 2026
Not needed for this change per review feedback.
Dev-iL
reviewed
Aug 1, 2026
Dev-iL
left a comment
Collaborator
There was a problem hiding this comment.
Couple of suggestions.
Other than that, if AI was used in making this PR, please add a disclosure section to the PR description, per the accepted format.
| # it dynamically, invisible to mypy's static checks here), so read it via | ||
| # ``getattr`` instead of a direct attribute access. | ||
| name = tbl.__tablename__ | ||
| tbl_schema = getattr(tbl, "__table__").schema |
Collaborator
There was a problem hiding this comment.
Consider using the local_table property available via SQLA's inspect api instead of getattr. I am not 100% sure about this, but prrhaps it could satisfy mypy, so type safety is retained.
| if not sep: | ||
| tbl_schema, name = None, tbl | ||
| else: | ||
| # A mapped class already carries its configured schema (e.g. via |
Collaborator
There was a problem hiding this comment.
I would trim the entire comment down to a few words or even remove it compeletely and let users refer to the PR for more context.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When
sql_alchemy_schemais set,reflect_tables()derived the schema to reflect a mapped class's table by splitting its bare__tablename__on a dot, which discards the schema entirely for classes likeLogTemplatewhose table name has no dot.synchronize_log_template()then looked up the reflected table by that same bare name, which also does not match how SQLAlchemy keys schema-qualified tables in the reflected metadata. Together these left thelog_templatetable unrecognized under a custom schema, causing triggering a Dag to fail with aTypeError.This derives the schema from the mapped class's own configured table instead of parsing its name, and looks the reflected table up by its schema-qualified key.
closes: #47300