Handle both SubDagOperator classes#1196
Merged
jlowin merged 1 commit intoapache:masterfrom Apr 1, 2016
Merged
Conversation
jlowin
force-pushed
the
subdag_class
branch
3 times, most recently
from
304bb8a to
756b42e
Compare
Member
|
Oh interesting, I thought both would have the same identity... Should we go with |
Member
Author
|
@mistercrunch yes, I like that! It gets around the import issue. But since it potentially includes other user-created tasks that could be called if (
task.__class__.__name__ == 'SubDagOperator'
and hasattr(task, 'subdag')): |
`airflow.operators.SubDagOperator` and `airflow.operators.subdag_operator.SubDagOperator` are NOT the same. Airflow needs to check against both classes to determine if a task is in fact a SubDagOperator. This is because of Airflow's import machinery. It is *probably* ok to check both classes with `isinstance()` but the behavior is surprising and to cover our bases we check for __class__.__name__ and a `subdag` attr. closes apache#1168
Member
|
As a longer term fix there might be a way to handle it in the importing function in |
|
Coverage decreased (-0.005%) to 66.368% when pulling de0b5d144412ed10936868e248d82217cc513831 on jlowin:subdag_class into 58029df on airbnb:master. |
|
|
mobuchowski
added a commit
to mobuchowski/airflow
that referenced
this pull request
Jan 4, 2022
* marquez-airflow bigquery stats Signed-off-by: Maciej Obuchowski <maciej.obuchowski@getindata.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
4 participants
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.
As described in #1168, Airflow validates subdags against
airflow.operators.SubDagOperator, which is NOT the same asairflow.operators.subdag_operator.SubDagOperator. Users who used the latter class got unexpected errors.Since this is a piece of Airflow’s internals, we can handle both cases explicitly (with this PR), but this isn’t a good long term fix for the many other cases where users could be using either of two possible Operator/Hook classes. See #1194.