[AIRFLOW-7022] Simplify DagFileProcessor.process_file method #7674
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
boring-cyborg
bot
added
the
area:Scheduler
mik-laj
changed the title
turbaszek
reviewed
Mar 10, 2020
ashb
reviewed
Mar 10, 2020
|
Travis is green again. |
|
Thanks for doing them as fixup commits @mik-laj -- I think we (comitters) should get more in the habit of it, makes re-review easier |
ashb
reviewed
Mar 11, 2020
tests/jobs/test_scheduler_job.py
Outdated
| @@ -1057,6 +1057,31 @@ def test_process_file_should_failure_callback(self): | |||
| self.assertEqual("Callback fired", content) | |||
| os.remove(callback_file.name) | |||
|
|
|||
| def test_should_parse_only_active_dags(self): | |||
There was a problem hiding this comment.
Suggested change
| def test_should_parse_only_active_dags(self): | |
| def test_should_parse_only_unpaused_dags(self): |
Active is another concept (the is_active column).
There was a problem hiding this comment.
👍 from me after this change is made
kaxil
reviewed
Mar 11, 2020
airflow/jobs/scheduler_job.py
Outdated
| @@ -845,11 +845,11 @@ def process_file( | |||
|
|
|||
| paused_dag_ids = DagModel.get_paused_dag_ids(dag_ids=dagbag.dag_ids) | |||
|
|
|||
| active_dags = [dag for dag_id, dag in dagbag.dags.items() if dag_id not in paused_dag_ids] | |||
| unpaaused_dags = [dag for dag_id, dag in dagbag.dags.items() if dag_id not in paused_dag_ids] | |||
There was a problem hiding this comment.
Suggested change
| unpaaused_dags = [dag for dag_id, dag in dagbag.dags.items() if dag_id not in paused_dag_ids] | |
| unpaused_dags = [dag for dag_id, dag in dagbag.dags.items() if dag_id not in paused_dag_ids] |
kaxil
reviewed
Mar 11, 2020
airflow/jobs/scheduler_job.py
Outdated
|
|
||
| simple_dags = self._prepare_simple_dags(active_dags, pickle_dags, session) | ||
| simple_dags = self._prepare_simple_dags(unpaaused_dags, pickle_dags, session) |
There was a problem hiding this comment.
Suggested change
| simple_dags = self._prepare_simple_dags(unpaaused_dags, pickle_dags, session) | |
| simple_dags = self._prepare_simple_dags(unpaused_dags, pickle_dags, session) |
kaxil
reviewed
Mar 11, 2020
airflow/jobs/scheduler_job.py
Outdated
|
|
||
| dags = self._find_dags_to_process(active_dags) | ||
| dags = self._find_dags_to_process(unpaaused_dags) |
There was a problem hiding this comment.
Suggested change
| dags = self._find_dags_to_process(unpaaused_dags) | |
| dags = self._find_dags_to_process(unpaused_dags) |
Codecov Report
@@ Coverage Diff @@
## master #7674 +/- ##
==========================================
- Coverage 86.84% 86.21% -0.63%
==========================================
Files 897 904 +7
Lines 42806 43740 +934
==========================================
+ Hits 37173 37710 +537
- Misses 5633 6030 +397
Continue to review full report at Codecov.
|
kaxil
approved these changes
Mar 12, 2020
ashb
approved these changes
Mar 12, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
NOTE FOR REVIEWERS: This PR contains several commits. Please review each commit in turn.
This method is too long, so I did the following changes:
Issue link: AIRFLOW-7022
Make sure to mark the boxes below before creating PR: [x]
[AIRFLOW-NNNN]. AIRFLOW-NNNN = JIRA ID** For document-only changes commit message can start with
[AIRFLOW-XXXX].In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.