[AIRFLOW-5136] Fix Bug with Incorrect template_fields in DataProc{*} …

[kaxil]

…Operators

Make sure you have checked all steps below.

Jira

• My PR addresses the following Airflow Jira issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"
  • https://issues.apache.org/jira/browse/AIRFLOW-5136

Description

• Here are some details about my PR, including screenshots of any UI changes:

Looks like a bad cherry-pick: a87310b

Traceback (most recent call last):
File "/usr/local/virtualenv/airflow/local/lib/python2.7/site-packages/airflow/models/dagbag.py", line 389, in collect_dags safe_mode=safe_mode)
File "/usr/local/virtualenv/airflow/local/lib/python2.7/site-packages/airflow/models/dagbag.py", line 253, in process_file self.bag_dag(dag, parent_dag=dag, root_dag=dag)
File "/usr/local/virtualenv/airflow/local/lib/python2.7/site-packages/airflow/models/dagbag.py", line 339, in bag_dag self.bag_dag(subdag, parent_dag=dag, root_dag=root_dag)
File "/usr/local/virtualenv/airflow/local/lib/python2.7/site-packages/airflow/models/dagbag.py", line 326, in bag_dag dag.resolve_template_files()
File "/usr/local/virtualenv/airflow/local/lib/python2.7/site-packages/airflow/models/dag.py", line 706, in resolve_template_files t.resolve_template_files()
File "/usr/local/virtualenv/airflow/local/lib/python2.7/site-packages/airflow/models/baseoperator.py", line 699, in resolve_template_files content = getattr(self, attr)
AttributeError: 'DataProcPySparkOperator' object has no attribute 'dataproc_pyspark_jars'

Tests

• My PR adds the following unit tests OR does not need testing for this extremely good reason:

Commits

• My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
  1. Subject is separated from body by a blank line
  2. Subject is limited to 50 characters (not including Jira issue reference)
  3. Subject does not end with a period
  4. Subject uses the imperative mood ("add", not "adding")
  5. Body wraps at 72 characters
  6. Body explains "what" and "why", not "how"

Documentation

• In case of new functionality, my PR adds documentation that describes how to use it.
  • All the public functions and the classes in the PR contain docstrings that explain what it does
  • If you implement backwards incompatible changes, please leave a note in the Updating.md so we can assign it to a appropriate release

Code Quality

• Passes flake8

[kaxil]

Targeted towards 1.10-test branch

[OmerJog]

@kaxil I think we are missing a test like this:
https://github.com/apache/airflow/blob/master/tests/contrib/operators/test_gcp_compute_operator.py#L71-L93
to confirm that templated fields are working and actual templating is being done
Such test would have prevented the bad cherry picked doesn't it?

[potiuk]

Yeah. @OmerJog is right. We will add tests for that for all GCP operators. We are in the process of unifying all GCP operators so I added https://issues.apache.org/jira/browse/AIRFLOW-5137 to cover that.

[ryanyuan]

@kaxil @potiuk @ashb Any ideas on why the changes in dataproc_operator.py in my GCP DLP commit?

[potiuk]

@ryanyuan - what do you mean ?

[ryanyuan]

@ryanyuan - what do you mean ?

@potiuk Forgot to paste a reference. Please see the following code.

https://github.com/apache/airflow/blame/222c6ac45de54ed0398645c1d456a592e194325b/airflow/contrib/operators/dataproc_operator.py#L20

A lot of changes to that file were put into my DLP commit.

[potiuk]

@ryanyuan From what I see it's quite OK. 222c6ac is a cherry-pick from master to v1-10-test branch. It contains the same changes as the original commit 6ef0e37 but applied to v1-10-test branch (some conflicts were resolved). In case you are not aware - the v1-10-test branch is completely separated from master - the current process we have with it that we select commits that we want to merge to 1.10.* and cherry-pick them to that branch. Those two branches are hundreds of commits different (but similar enough that cherry-picks were usually easy so far). What you likely see is the result of diff of your "master" with the v1-10-test branch. I hope it explains it :)

[potiuk]

And this commit here is against v1-10-test branch directly to fix the problems with bad merge - where templated field names changed :(

[kaxil]

@OmerJog - Yes you are right. I am merging this one as it is as now we have an issue open for adding test to all operators for templated fields which would be target to Master + Cherrypicked to v1-10-test

@ryanyuan - Jarek explained it well. It is just a case of bad cherry-pick that happens as both branches are diverging. We should add tests that cover it like Omer mentioned.

[ryanyuan]

@potiuk That makes sense. Thanks for the explanation!

[ghost]

@kaxil , how soon will this fix make it to a 1.10.x release? Will it have to wait until 1.10.5 is released? Currently dataproc integration is broken in 1.10.4.

[kaxil]

We will release 1.10.5 with fixes to Dataproc Operators, hopefully by next week. We are going to release 1.10.5 just to fix Daatproc &GCS integrations so it should be fairly quick

…

On Tue, Aug 27, 2019 at 7:21 PM Andrey Klochkov ***@***.***> wrote: @kaxil <https://github.com/kaxil> , how soon will this fix make it to a 1.10.x release? Will it have to wait until 1.10.5 is released? Currently dataproc integration is broken in 1.10.4. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5751?email_source=notifications&email_token=ACDHIJWD57FLBPG4AJ6U5FDQGVWDBA5CNFSM4IKCB3LKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5IVGUQ#issuecomment-525423442>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACDHIJXQBJ72MBY77UKSL6TQGVWDBANCNFSM4IKCB3LA> .