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.

Sign up for GitHub

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

Jump to bottom

[AIRFLOW-4420] Backfill respects task_concurrency #5221

Merged
merged 1 commit into from
May 9, 2019
Merged

[AIRFLOW-4420] Backfill respects task_concurrency #5221

merged 1 commit into from
May 9, 2019

Conversation

milton0825
Copy link
Contributor

@milton0825 milton0825 commented May 1, 2019 •
edited
Loading

Make sure you have checked all steps below.

Jira

Description

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

Ensure that Airflow backfill respects task_concurrency.

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

@milton0825 milton0825 force-pushed the backfill-respects-task-concurrency branch from a3a6111 to 389a749 Compare May 1, 2019 05:42
@milton0825 milton0825 marked this pull request as ready for review May 1, 2019 06:15
@milton0825 milton0825 force-pushed the backfill-respects-task-concurrency branch from 389a749 to 45c2f14 Compare May 1, 2019 06:16
@codecov-io
Copy link

codecov-io commented May 1, 2019 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5221      +/-   ##
==========================================
+ Coverage   78.66%   78.67%   +<.01%     
==========================================
  Files         470      470              
  Lines       30008    30015       +7     
==========================================
+ Hits        23607    23613       +6     
- Misses       6401     6402       +1
Impacted Files Coverage Δ
airflow/exceptions.py 100% <100%> (ø) ⬆️
airflow/jobs.py 78.43% <100%> (+0.09%) ⬆️
airflow/models/taskinstance.py 92.42% <0%> (-0.18%) ⬇️

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 a6daeb5...720820e. Read the comment docs.

@milton0825 milton0825 force-pushed the backfill-respects-task-concurrency branch from 45c2f14 to 7aec650 Compare May 7, 2019 22:15
@milton0825
[AIRFLOW-4420] Backfill respects task_concurrency
720820e 
Ensure that backfill respects task_concurrency.
That is, the number of concurrent running tasks
across DAG runs should not exceed task_concurrency.
@milton0825 milton0825 force-pushed the backfill-respects-task-concurrency branch from 7aec650 to 720820e Compare May 7, 2019 23:07
@milton0825
Copy link
Contributor Author

PTAL @feng-tao @potiuk

potiuk
potiuk reviewed May 8, 2019
View reviewed changes
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Very nice. Can I have one more small request though. It seems that airflow documentation is not very good about explaining those two different concurrency limits - maybe you can update the documentation slightly explaining those ?

feng-tao
feng-tao approved these changes May 8, 2019
View reviewed changes
Copy link
Member

@feng-tao feng-tao left a comment

Choose a reason for hiding this comment

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

lgtm, I think you could improve the entry in the https://github.com/apache/airflow/blob/master/docs/faq.rst on different concurrency.

@milton0825
Copy link
Contributor Author

milton0825 commented May 9, 2019 •
edited
Loading

Added documentation around task_concurrency in: #5262

@feng-tao
Copy link
Member

feng-tao commented May 9, 2019

lgtm, thanks @milton0825 for adding the doc.

@feng-tao feng-tao merged commit e187625 into apache:master May 9, 2019
JCoder01 pushed a commit to JCoder01/airflow that referenced this pull request May 14, 2019
@milton0825
[AIRFLOW-4420] Backfill respects task_concurrency (apache#5221)
805804d 
Ensure that backfill respects task_concurrency.
That is, the number of concurrent running tasks
across DAG runs should not exceed task_concurrency.
@milton0825 milton0825 deleted the backfill-respects-task-concurrency branch May 18, 2019 07:24
ashb pushed a commit to ashb/airflow that referenced this pull request May 28, 2019
@milton0825 @ashb
[AIRFLOW-4420] Backfill respects task_concurrency (apache#5221)
e824cc7 
Ensure that backfill respects task_concurrency.
That is, the number of concurrent running tasks
across DAG runs should not exceed task_concurrency.

(cherry picked from commit e187625)
andriisoldatenko pushed a commit to andriisoldatenko/airflow that referenced this pull request Jul 26, 2019
@milton0825 @andriisoldatenko
[AIRFLOW-4420] Backfill respects task_concurrency (apache#5221)
8d72377 
Ensure that backfill respects task_concurrency.
That is, the number of concurrent running tasks
across DAG runs should not exceed task_concurrency.
wmorris75 pushed a commit to modmed/incubator-airflow that referenced this pull request Jul 29, 2019
@milton0825
[AIRFLOW-4420] Backfill respects task_concurrency (apache#5221)
49cc632 
Ensure that backfill respects task_concurrency.
That is, the number of concurrent running tasks
across DAG runs should not exceed task_concurrency.
dharamsk pushed a commit to postmates/airflow that referenced this pull request Aug 8, 2019
@milton0825 @dharamsk
[AIRFLOW-4420] Backfill respects task_concurrency (apache#5221)
e0f11fd 
Ensure that backfill respects task_concurrency.
That is, the number of concurrent running tasks
across DAG runs should not exceed task_concurrency.

(cherry picked from commit e187625)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@potiuk potiuk potiuk left review comments

@feng-tao feng-tao feng-tao approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@milton0825 @codecov-io @feng-tao @potiuk