[AIRFLOW-984] enable subclassing of SubDagOperator

[patrickmckenna]

Please accept this PR that addresses the following issues:

• https://issues.apache.org/jira/browse/AIRFLOW-984

This essentially (though not technically) reverts #1196. The issue referenced in that PR no longer exists, so I couldn't get the full context for why that change was made. However, this PR still addresses what I understand to be the original problem, while also letting users create subclasses of SubDagOperator that work as expected.

[jlowin]

@patrickmckenna from airflow.operators import SubDagOperator is deprecated, and that's why your tests are failing (it will work out of the box for backwards-compatibility reasons but unit tests are run with compatibility turned off)

[patrickmckenna]

@jlowin Travis is currently erroring out, but I believe 3393003 takes care of this.

Is this style of importing deprecated more generally? Where should I look for the latest discussion on this? Thanks.

[jlowin]

Importing any operator from airflow.operators is being deprecated because of creeping complexity. You should be seeing a warning like this:

In [1]: import airflow
[2017-03-15 21:05:33,021] {__init__.py:56} INFO - Using executor LocalExecutor

In [2]: from airflow.operators import SubDagOperator
/Users/jlowin/git/airflow/airflow/utils/helpers.py:406: DeprecationWarning: Importing SubDagOperator directly from <module 'airflow.operators' from '/Users/jlowin/git/airflow/airflow/operators/__init__.py'> has been deprecated. Please import from '<module 'airflow.operators' from '/Users/jlowin/git/airflow/airflow/operators/__init__.py'>.[operator_module]' instead. Support for direct imports will be dropped entirely in Airflow 2.0.
  DeprecationWarning)

(looks like the module names got a little mangled at some point!)

So after thinking about this some more, I gave you some bad advice. You're caught in a tight place -- trying to adjust some deprecated behavior that's nonetheless maintained for backwards-compatibility reasons - so we need to awkwardly support both modes.

Sorry to ask you to revise one more time, but I think the right way to do this is to leave the existing check and add an additional or isinstance(task, SubDagOperator) to the check, where SubDagOperator is properly imported as you're doing now. This way the legacy check is maintained for backwards compatibility and your way handles it properly going forward (and bonus points for putting a #TODO remove in Airflow 2.0 over the legacy part of the check.)

Sorry for the confusion!

[codecov-io]

Codecov Report

Merging #2152 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2152      +/-   ##
==========================================
+ Coverage   67.17%   67.17%   +<.01%     
==========================================
  Files         142      142              
  Lines       10796    10798       +2     
==========================================
+ Hits         7252     7254       +2     
  Misses       3544     3544

Impacted Files	Coverage Δ
airflow/models.py	86.92% <100%> (+0.01%)	✅
airflow/configuration.py	84.42% <0%> (ø)	✅

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c44e200...99deabf. Read the comment docs.

[patrickmckenna]

Hey, it happens! Thanks for the clarification.

On a related note, what is the latest stable release? PyPI shows 1.7.1.3. JIRA doesn't have a release date for that version and shows 2 unresolved issues for it; it also shows 1.8.0 as having a release date of 2017-02-02 and with 0 unresolved issues. There is a Git tag for 1.8.0, but that tag seems to be getting overwritten regularly. (For example, it used to point at 310fb58, but was changed recently to point at f4760c3.) (I'm unfamiliar with Apache's conventions, so am uncertain what source to consider the canonical reference.)

[jlowin]

Latest stable is 1.7.1.3. Version 1.8.0 is being voted for release right now, and has gone through a number of release candidates (hence the tag changes). The likely confusion in JIRA is because 1.8.0 is (or will be soon) the first release since Airflow joined Apache.

[jlowin]

LGTM, thanks @patrickmckenna and sorry again for the confusion!