Codecov Report

Merging #7456 into master will decrease coverage by 0.17%.
The diff coverage is 99%.

@@            Coverage Diff             @@
##           master    #7456      +/-   ##
==========================================
- Coverage   86.79%   86.62%   -0.18%     
==========================================
  Files         887      887              
  Lines       41976    41991      +15     
==========================================
- Hits        36432    36373      -59     
- Misses       5544     5618      +74

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 4e0e2f0...0306223. Read the comment docs.

@potiuk Great to hear you like it. I have provided an Updating.md memo regarding these changes and will squash/rebase my commits before the merge.

Plus you will need to rebase it @msb217 !

I love this change! <3

Awesome work, congrats on your first merged pull request!

Thanks @msb217 ! First commit and so important for us !

Thanks @potiuk , I am happy to contribute and hope that I can help with more issues in the future!

This is going to need a change to the vast majority of dag in existence - our own docs http://airflow.apache.org/docs/stable/tutorial.html say from airflow import DAG .

Is there any clever tricks we can do to have that import still work but only import models etc when first accessed?

I like the change but also with @ashb here, it's going to break a lot of workflows which import from airflow import DAG. We should at least show deprecation warnings.

@ashb I don't understand your question? I know of the magic __import__ function for changing import behaviour, but don't know if it can be applied to your question?

For python 3 we can possibly do https://www.python.org/dev/peps/pep-0562/ do have a airflow.DAG still work, but issue a depreciation warning. I'll take a look at a warning on Monday

One thing to consider.

I would like to see if we can think slightly differently here.

This is a change that is super easy to add to automated "migration assistant" tool that we will have to do anyway. I think we will have far more issues to deal with in terms of backwards compatibility (for example bash_operator and python_operator have moved).

I think personally that with the 2.0 migration we should simply make people aware that they have to migrate using migration tool (or manually). Otherwise they will have a false sense of having "compatible" dags. I think telling people they will be able to move their dags to 2.0 and they will continue to work as usual, is a bit miscommunication (or even a bit of a lie - we know it's not going to work in a number of cases).

Migration to 2.0 will be anyway pain for many people but they can deal with one pain at at time - i.e. DAG migration first, then migrate to 2.0 Airflow infrastructure.

We can make it easier for people to migrate to 2.0 - if they will have the new "back-ported" providers packages installed - the migration to the "both 1.10 and 2.0" compatible dags might be done automatically even BEFORE migration to 2.0 and our tool should be able to tell how "ready" you are to migrate to 2.0. For now - I believe none of the changes we've done prevents us from having Dags that are both 1.0 and 2.0 compatible (and if we find one - we can still easily fix it in master to keep that possibility).

The "migration assistant" can produce both - 1.10 and 2.0 compatible DAGs (for example imports changed to "airflow.models.DAG" etc. This should be very easy with tools like Bowler : https://pybowler.io/ - we used it successfully to refactor tons of python code (basically all providers code!) and we can easily produce dag-migration assistant that can be used by people.

I think we have the easiest migration path for our users to follow:

[1.10] py2 (those still on py2) -> 
     [1.10] py3 -> 
         [1.10] py3 + providers (incremental) -> 
            [1.10] py3 + providers + 2.0-compatible DAGs (incremental) -> 
                [2.0] 

This way while migrating the users will have to deal with one problem at a time and the most difficult part (adapting DAGs) can be mostly automated and can be done incrementally while they are still running 1.10. The fact that this is incremental and that they do not have to migrate everything at once is really appealing for many users I think.

I've thought about this a bit more, and I'm coming at this from a different point of view than simple migration - that of user facing API and what we should expose to them.

Also the easier we make the migration the more likely it is to not leave users behind. So if they can run old dags with new Airflow so much the better. Lets take a lesson from Django here, they've thought long and hard about how to do migrations and upgrades. In summary:

If code works in one version without warnings, it MUST work in the next version but can issue deprecation warnings, and can only be removed in the release after that.

Also to me, from airflow import DAG is nicer -- why should the DAG authors have to care about the exact internal implementation of where we've chosen to move this class to now?. Airflow and DAGS go hand in hand -- with PEP-562 we can have lazily imported classes but still present a nice interface to users. And plus we can then re-arrange things later under the hood without having to affect our users. After all, what is the harm if the importing everything is solved?

My proposal here:

• Add in a _getattr_ based lazy import for DAG to airflow/init.py module
• Do NOT issue a deprecation warning for this
• Revert the change to all the imports in example dags etc so that those do from airflow import DAG.

I'm raising the point about what should our API be on the mailing list.

Plus this PR didn't (yet) have much of an impact on import times - #7505 :)

Not yet indeed :).

Let's talk at the devlist indeed. I understand where you are coming from, but I think it's not good idea for the users in the future to switch to that.

While I understand (and concur) we might want to add PEP-562 behavior for backwards compatibility, I don't think it should be default behavior for examples and future users:

• explicit is better than implicit
• IDE support and mypy will go crazy (I believe - I would like to check it when it's done)

I think a lot of our users are not super-hackers and they use IDEs to write the code. If the PEP-562 results with having no autocomplete/verification of proper import, then it's a no-go for me.