[AIRFLOW-4686] Make dags Pylint compatible

[feluelle]

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-4686
  • In case you are fixing a typo in the documentation you can prepend your commit with [AIRFLOW-XXX], code changes always need a Jira issue.
  • In case you are proposing a fundamental code change, you need to create an Airflow Improvement Proposal (AIP).
  • In case you are adding a dependency, check if the license complies with the ASF 3rd Party License Policy.

Description

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

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

[feluelle]

@potiuk I do not understand why I have this test failing when I didn't change anything there.

======================================================================

21) FAIL: test_execute (tests.contrib.operators.test_cassandra_to_gcs_operator.CassandraToGCSTest)

----------------------------------------------------------------------

   Traceback (most recent call last):

    /usr/local/lib/python3.5/unittest/mock.py line 1159 in patched

      return func(*args, **keywargs)

    tests/contrib/operators/test_cassandra_to_gcs_operator.py line 53 in test_execute

      test_bucket, schema, TMP_FILE_NAME, "application/json", gzip

    /usr/local/lib/python3.5/unittest/mock.py line 794 in assert_called_with

      raise AssertionError(_error_message()) from cause

   AssertionError: Expected call: upload('test-bucket', 'schema.json', 'temp-file', 'application/json', True)

   Actual call: upload('test-bucket', 'data.json', 'temp-file', 'application/json', True)

The test was added here and it passed all tests.

[potiuk]

Yeah. I think it's a flaky test - passing sometimes - I saw it failing in master as well. I think we need to remove it for now.

[potiuk]

It looks like there are two calls - one for schema.json and one for data.json and sometimes one comes first, sometimes the other :).

[potiuk]

Yeah. We are experiencing this: https://stackoverflow.com/questions/7242433/asserting-successive-calls-to-a-mock-method - assert_called_with only check the last call and there are two - and they race each other

[potiuk]

I will do a quick fix

[feluelle]

Yes, I just noticed that, too, that I 've never used assert_called_with.

I use assert_has_calls for multiple calls and for single calls I use assert_called_once_with

[feluelle]

We could use assert_has_calls with any_order set to True.

[feluelle]

Oh that's exactly what the link is saying :D

[potiuk]

Yep. Just submitting this fix :)

       call_schema = call(test_bucket, schema, TMP_FILE_NAME, "application/json", gzip)
        call_data = call(test_bucket, filename, TMP_FILE_NAME, "application/json", gzip)
        mock_upload.assert_has_calls([call_schema, call_data], any_order=True)

[potiuk]

Fix here: #5758

[feluelle]

We should make a pre-commit hook to check if we have assert_called_with in our /tests :P

I've integrated the pre-commit framework for our custom plugins - works like a charm :)

[potiuk]

Oh yeah. That's coming :)

[feluelle]

@potiuk is this fine now? :P ..or may want @ashb have a quick look at this? :)

[mik-laj]

One small comment and today I hope merge this PR.

[mik-laj]

@feluelle Thanks. Great work. Fixing pylint errors is very important. This will allow for faster development of the project.