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

Further validation that only task commands are run by executors #9240

Merged
merged 4 commits into from
Jun 12, 2020
Merged

Further validation that only task commands are run by executors #9240

merged 4 commits into from
Jun 12, 2020

Conversation

kaxil
Copy link
Member

@kaxil kaxil commented Jun 12, 2020 •
edited
Loading

#9178 added a check to raise an exception if commands don't start with airflow tasks run. However, the tests were passing on CI (maybe because of cache) but were failing on Breeze

Make sure to mark the boxes below before creating PR: [x]

  • Description above provides context of the change
  • Unit tests coverage for changes (not needed for documentation changes)
  • Target Github ISSUE in description if exists
  • Commits follow "How to write a good git commit message"
  • Relevant documentation is updated including usage instructions.
  • I will engage committers as explained in Contribution Workflow Example.

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.

@kaxil kaxil requested a review from ashb June 12, 2020 00:20
@boring-cyborg boring-cyborg bot added the area:Scheduler including HA (high availability) scheduler label Jun 12, 2020
@kaxil kaxil added this to the Airflow 1.10.11 milestone Jun 12, 2020
ashb
ashb reviewed Jun 12, 2020
View reviewed changes
tests/executors/test_kubernetes_executor.py Outdated
@@ -205,7 +205,7 @@ def test_run_next_exception(self, mock_get_kube_client, mock_kubernetes_job_watc
try_number = 1
kubernetes_executor.execute_async(key=('dag', 'task', datetime.utcnow(), try_number),
queue=None,
command='command',
command=["airflow", "version"],
Copy link
Member

Choose a reason for hiding this comment

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

This is still an invalid command. Or is the change really just to make it a list?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated in 3c61dfd

@kaxil kaxil requested a review from ashb June 12, 2020 11:46
@kaxil
Fix Airflow Tests after apache#9178
bbd88f7 
apache#9178 added check to raise exception if commands don't start with `airflow tasks run`. However the test were passing on CI (mabe because of cache) but were failing on Breeze
@kaxil
fixup! Fix Airflow Tests after apache#9178
ded9069
@kaxil
fixup! fixup! Fix Airflow Tests after apache#9178
fbadc2c
@ashb ashb changed the title Fix Airflow Tests after #9178 Further Validation that only task commands are run by executors Jun 12, 2020
@ashb ashb changed the title Further Validation that only task commands are run by executors Further validation that only task commands are run by executors Jun 12, 2020
@kaxil kaxil merged commit 99c534e into apache:master Jun 12, 2020
@kaxil kaxil deleted the fix-tests branch June 12, 2020 21:28
kaxil added a commit to astronomer/airflow that referenced this pull request Jun 15, 2020
@kaxil
Further validation that only task commands are run by executors (apac…
c96af8e 
…he#9240)

(cherry-picked from 99c534e)
kaxil added a commit that referenced this pull request Jun 15, 2020
@kaxil
Further validation that only task commands are run by executors (#9240)
7d332c5 
(cherry-picked from 99c534e)
kaxil added a commit to astronomer/airflow that referenced this pull request Jun 16, 2020
@kaxil
Further validation that only task commands are run by executors (apac…
9405e0b 
…he#9240)

(cherry-picked from 99c534e)

(cherry picked from commit 3ceb8b0501b4b5f9f9f6bef04db11ea0422a114f)
kaxil added a commit to astronomer/airflow that referenced this pull request Jun 16, 2020
@kaxil
Further validation that only task commands are run by executors (apac…
7e31614 
…he#9240)

(cherry-picked from 99c534e)

(cherry picked from commit 3ceb8b0501b4b5f9f9f6bef04db11ea0422a114f)
potiuk pushed a commit that referenced this pull request Jun 29, 2020
@kaxil @potiuk
Further validation that only task commands are run by executors (#9240)
10d808b 
(cherry-picked from 99c534e)
kaxil added a commit that referenced this pull request Jul 1, 2020
@kaxil
Further validation that only task commands are run by executors (#9240)
435b072 
(cherry-picked from 99c534e)
cfei18 pushed a commit to cfei18/incubator-airflow that referenced this pull request Mar 5, 2021
@kaxil
Further validation that only task commands are run by executors (apac…
37a32b8 
…he#9240)

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

@ashb ashb ashb approved these changes

Assignees
No one assigned
Labels
area:Scheduler including HA (high availability) scheduler
Projects
None yet
Milestone
Airflow 1.10.11
Development

Successfully merging this pull request may close these issues.
2 participants
@kaxil @ashb