Repository navigation
[v3-4-test] Fix log template sync failing when a custom SQL Alchemy schema is configured (#70793) - #75194
Merged
Conversation
…chema is configured (#70793) * Fix log template sync failing when a custom SQL Alchemy schema is configured 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. * Add newsfragment for log template custom schema fix * Remove newsfragment for log template custom schema fix Not needed for this change per review feedback. * Use inspect().local_table instead of getattr for the mapped class's schema getattr(tbl, "__table__") worked around __table__ not being declared on MappedClassProtocol, but SQLAlchemy's inspect() already exposes the same table through a properly typed Mapper.local_table, so the getattr and its explanatory comment are no longer needed. (cherry picked from commit 6aee277) Co-authored-by: Aaryan Mahajan <aaryanmhjn@gmail.com>
1 task done
vatsrahul1001
marked this pull request as ready for review
October 20, 2026 08:37
vatsrahul1001
approved these changes
Oct 20, 2026
Contributor
Author
|
Hi maintainer, this PR was merged without a milestone set.
|
vatsrahul1001
added a commit
that referenced
this pull request
Oct 20, 2026
…chema is configured (#70793) (#75194) * Fix log template sync failing when a custom SQL Alchemy schema is configured 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. * Add newsfragment for log template custom schema fix * Remove newsfragment for log template custom schema fix Not needed for this change per review feedback. * Use inspect().local_table instead of getattr for the mapped class's schema getattr(tbl, "__table__") worked around __table__ not being declared on MappedClassProtocol, but SQLAlchemy's inspect() already exposes the same table through a properly typed Mapper.local_table, so the getattr and its explanatory comment are no longer needed. (cherry picked from commit 6aee277) Co-authored-by: Aaryan Mahajan <aaryanmhjn@gmail.com> Co-authored-by: Rahul Vats <43964496+vatsrahul1001@users.noreply.github.com>
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_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.
Add newsfragment for log template custom schema fix
Remove newsfragment for log template custom schema fix
Not needed for this change per review feedback.
getattr(tbl, "table") worked around table not being declared on
MappedClassProtocol, but SQLAlchemy's inspect() already exposes the same
table through a properly typed Mapper.local_table, so the getattr and its
explanatory comment are no longer needed.
(cherry picked from commit 6aee277)
Co-authored-by: Aaryan Mahajan aaryanmhjn@gmail.com