Repository navigation
fix: scheduled docs with latest_partition fail to run - #1101
Merged
Merged
Conversation
czgu
reviewed
Dec 14, 2022
|
|
||
|
|
||
| def get_templated_query_env(engine_id: int): | ||
| def get_templated_query_env(engine_id: int, session=None): |
Collaborator
There was a problem hiding this comment.
just have session without defaulting to None (otherwise it would fail)
|
|
||
| def render_templated_query( | ||
| query: str, variables: Dict[str, str], engine_id: int | ||
| query: str, variables: Dict[str, str], engine_id: int, session=None |
czgu
approved these changes
Dec 14, 2022
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.
Issue
A user reported that one of their scheduled doc failed to run with status stuck on
RUNNING, while manual run each query cell succeeds. (manual run the schedule also fails).Findings
[2022-12-13 18:44:36,616: ERROR/ForkPoolWorker-210079] Task run_data_doc_216961[532b190e-f6c3-4710-8a84- bc6f2701f904] raised unexpected: DetachedInstanceError('Instance <DataCell at 0x7f040851f250> is not bound to a Session; attribute refresh operation cannot proceed')latest_partitionlatest_partition. ANDCause
The issue was introduced since PR-1073. To explain the cause, we have to understand how the session works in Sqlalchemy.
detachedandexpireddetachedmeans that the object has been fully removed from the session. Any changes made to the object will not be committed to the db anymore.expiredmeans the content of the object has been expired, while the object could be still attached to a session or maybe not.with DBSession()orwith_session, all objects will bedetachedfrom the session, but may benot expiredsession.commitwill cause all objects of a session to beexpired, butnot detached.DetachedInstanceErrorBack to issue of the
run_datadocfunction, the code flow is like belowBefore the PR-1073, although the query cell also has been detached from the session, but as it's not expired either(didn't call commit), so it can still access its attribute, which did cause any issue.
Fix
When nesting db sessions, we should avoid the current session got closed which makes all the objects to be detached.
In our case, we'll pass the current session down to
render_templated_query, so thatcreate_get_latest_partitionwill use this same session when it's already insidewith DBSession()context and not close it. If it doesn't havesessionpassed over, it can create its own bywith_session.