Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AIRFLOW-6171] Apply .airflowignore to correct subdirectories #6784

Merged
merged 6 commits into from
Dec 12, 2019

Conversation

ghost
Copy link

@ghost ghost commented Dec 10, 2019

Fix the defect that applied .airflowignore rules from one subdirectory
to all other subdirectories scanned later.

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-XXX
    • 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

Fix the defect that applied .airflowignore rules from one subdirectory
to all other subdirectories scanned later.
'start_date': DEFAULT_DATE,
}

dag = DAG(dag_id='test_dag_under_subdir2', default_args=args)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't actually need a dag for the test to work, so this file can be mostly empty.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be almost empty but this is under the dags folder and we're making this file look like a dag to Airflow so it seems to make sense to place an actual test dag here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes and no - we're only testing the ignore behaviour, which we test "in isolation" outside of the rest of the scheduler. Anyway, let's put schedule_interval=None then please so that this doesn't actually run?

Copy link
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good, small change requested though please.

@codecov-io
Copy link

codecov-io commented Dec 12, 2019

Codecov Report

Merging #6784 into master will decrease coverage by 0.23%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6784      +/-   ##
==========================================
- Coverage   84.53%   84.29%   -0.24%     
==========================================
  Files         672      672              
  Lines       38153    38219      +66     
==========================================
- Hits        32252    32218      -34     
- Misses       5901     6001     +100
Impacted Files Coverage Δ
airflow/utils/file.py 91.46% <100%> (ø) ⬆️
airflow/kubernetes/volume_mount.py 44.44% <0%> (-55.56%) ⬇️
airflow/kubernetes/volume.py 52.94% <0%> (-47.06%) ⬇️
airflow/kubernetes/pod_launcher.py 45.25% <0%> (-46.72%) ⬇️
airflow/kubernetes/refresh_config.py 50.98% <0%> (-23.53%) ⬇️
...rflow/contrib/operators/kubernetes_pod_operator.py 78.2% <0%> (-20.52%) ⬇️
airflow/contrib/operators/ssh_operator.py 83.33% <0%> (-1.29%) ⬇️
airflow/executors/kubernetes_executor.py 57.29% <0%> (-0.64%) ⬇️
airflow/gcp/hooks/bigquery.py 70.71% <0%> (-0.14%) ⬇️
...flow/contrib/operators/gcp_cloud_build_operator.py 100% <0%> (ø) ⬆️
... and 5 more

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 53aa975...e47ffad. Read the comment docs.

@ashb ashb merged commit 702005f into apache:master Dec 12, 2019
kaxil pushed a commit that referenced this pull request Dec 13, 2019
kaxil pushed a commit that referenced this pull request Dec 13, 2019
kaxil pushed a commit that referenced this pull request Dec 17, 2019
galuszkak pushed a commit to FlyrInc/apache-airflow that referenced this pull request Mar 5, 2020
…#6784)

Fix the defect that applied .airflowignore rules from one subdirectory
to all other subdirectories scanned later.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants