Move Kubernetes modules out of contrib to match 2.0 layout #8904

dimberman · 2020-05-18T19:41:04Z

Moves k8s internal files out of the contrib folder without moving K8sPodOperator avoid any breaking changes

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.

dimberman · 2020-05-18T19:42:02Z

Many k8soperator/k8sexecutor PRs need to be essentially recreated from scratch because git can no longer merge. I'm hoping to make a few PRs like this to bring 1-10 and master closer together

ashb

On my phone right now so can't easily check, but I think we need an import-shim to make any old imports in end-user code stay working

.github/workflows/ci.yml

dimberman · 2020-05-18T21:13:13Z

@ashb some point tomorrow I might need some help on this PR. I'm getting a bunch of flake8 failures that seem completely unrelated to any code changes I made so not sure if something is funky with 1-10-test

dimberman · 2020-05-18T21:22:55Z

@potiuk do you have any idea why all of these unrelated flake tests would fail?


tests/contrib/utils/base_gcp_system_test_case.py:89:0: F523 '...'.format(...) has unused arguments at position(s): 0
airflow/jobs/backfill_job.py:559:33: F523 '...'.format(...) has unused arguments at position(s): 0, 1
airflow/contrib/utils/gcp_field_validator.py:253:17: F523 '...'.format(...) has unused arguments at position(s): 1
airflow/operators/docker_operator.py:268:17: E741 ambiguous variable name 'l'
tests/contrib/operators/test_gcs_to_gcs_operator.py:290:21: F523 '...'.format(...) has unused arguments at position(s): 1
airflow/contrib/operators/file_to_wasb.py:60:13: F523 '...'.format(...) has unused arguments at position(s): 0, 1, 2
airflow/contrib/operators/qubole_check_operator.py:226:17: E122 continuation line missing indentation or outdented

kaxil · 2020-05-19T10:32:21Z

@potiuk do you have any idea why all of these unrelated flake tests would fail?


tests/contrib/utils/base_gcp_system_test_case.py:89:0: F523 '...'.format(...) has unused arguments at position(s): 0
airflow/jobs/backfill_job.py:559:33: F523 '...'.format(...) has unused arguments at position(s): 0, 1
airflow/contrib/utils/gcp_field_validator.py:253:17: F523 '...'.format(...) has unused arguments at position(s): 1
airflow/operators/docker_operator.py:268:17: E741 ambiguous variable name 'l'
tests/contrib/operators/test_gcs_to_gcs_operator.py:290:21: F523 '...'.format(...) has unused arguments at position(s): 1
airflow/contrib/operators/file_to_wasb.py:60:13: F523 '...'.format(...) has unused arguments at position(s): 0, 1, 2
airflow/contrib/operators/qubole_check_operator.py:226:17: E122 continuation line missing indentation or outdented

That is most likely caused by the new version release of flake8 on May 12 that may have introduced extra checks ! - https://pypi.org/project/flake8/

potiuk · 2020-05-19T11:11:18Z

Yep. It's very likely. I think we fixed similar problems in one of the commits in master. Looking at the requirements.txt Flake8 3.8.0 (and then 3.8.1 right after) has been released 12th of May. You simply need to fix those @dimberman

dimberman · 2020-05-19T13:11:09Z

Thanks @potiuk @kaxil just wanted to make sure I wasn't making the problem worse :)

kaxil

PR targets v1-10-test so needs to be backwards-compatible. So just needs to add original paths to keep it compatible otherwise LGTM

kaxil · 2020-05-19T17:15:53Z

airflow/contrib/operators/file_to_wasb.py

+            'Uploading {} to wasb://{} '
+            'as {}'.format(self.file_path, self.container_name, self.blob_name)


Was this changes because of a flake8 error ? We use %s format for logs so that the arguments are passed properly when logging levels are changed

Yes. This was because of flake8 errors.

@kaxil any thoughts on what to do here? change them or set ignore flags?

also if you're running .format on the string before it's even parsed does it even matter?

Coz of #4804 (comment)

@kaxil fixed

The fix here is to do

self.log.info( 'Uploading %s to wasb://%s as %s', self.file_path, self.container_name, self.blob_name)

That way if the logging level is disabled, no time is wasted formatting the string

@ashb If possible I'd like to avoid making changes unrelated to the commit on this PR (especially since it's going straight to 1-10). Should we make a separate PR for fixing these logs/flake8 tests?

Oh gotcha. I mean you are already changing this, so it's going to conflict anyway.

Potato potato.

airflow/contrib/operators/file_to_wasb.py

ashb · 2020-05-21T22:13:07Z

@dimberman Needs rebasing now please.

get static checks to pass

Considering that the k8s executor is now supported by core committers, we should move it from contrib to the primary executor directory. Co-authored-by: Daniel Imberman <daniel@astronomer.io>

dimberman requested review from ashb, bolkedebruin and kaxil May 18, 2020 19:41

boring-cyborg bot added area:docs area:Scheduler Scheduler or dag parsing Issues area:webserver Webserver related Issues labels May 18, 2020

ashb reviewed May 18, 2020

View reviewed changes

dimberman force-pushed the k8s-file-move branch 2 times, most recently from ebef081 to 14329ce Compare May 18, 2020 21:09

ashb reviewed May 18, 2020

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

dimberman force-pushed the k8s-file-move branch from 8c8d483 to d4030a3 Compare May 18, 2020 21:11

kaxil requested changes May 19, 2020

View reviewed changes

kaxil reviewed May 19, 2020

View reviewed changes

kaxil reviewed May 20, 2020

View reviewed changes

airflow/contrib/operators/file_to_wasb.py Outdated Show resolved Hide resolved

ashb approved these changes May 20, 2020

View reviewed changes

dimberman force-pushed the k8s-file-move branch from 9252955 to d71839c Compare May 20, 2020 16:16

kaxil approved these changes May 20, 2020

View reviewed changes

dimberman force-pushed the k8s-file-move branch 2 times, most recently from 31d03c6 to a49aebb Compare May 21, 2020 15:44

ashb changed the title ~~Move k8s files out of contrib~~ Move Kubernetes modules out of contrib to match 2.0 layout May 21, 2020

dimberman force-pushed the k8s-file-move branch 2 times, most recently from 15e7728 to c96119c Compare May 26, 2020 16:02

dimberman force-pushed the k8s-file-move branch from c96119c to 019e02b Compare May 28, 2020 16:40

dimberman force-pushed the v1-10-test branch from 59cac08 to 77e6311 Compare May 28, 2020 16:54

dimberman force-pushed the k8s-file-move branch from 019e02b to 24aca2f Compare May 28, 2020 16:54

dimberman force-pushed the v1-10-test branch 5 times, most recently from 3a4e73c to 2518ffb Compare May 29, 2020 13:54

dimberman force-pushed the k8s-file-move branch from 24aca2f to 161a2aa Compare May 29, 2020 14:06

dimberman force-pushed the v1-10-test branch from 2518ffb to 62e5696 Compare May 29, 2020 14:14

Move k8sexecutor out of contrib to closer match master

b101e2e

get static checks to pass

dimberman force-pushed the k8s-file-move branch from 161a2aa to b101e2e Compare May 29, 2020 15:05

dimberman merged commit 4daccda into apache:v1-10-test May 29, 2020

dimberman deleted the k8s-file-move branch May 29, 2020 15:44

xylar mentioned this pull request Jul 22, 2020

airflow v1.10.11 conda-forge/airflow-feedstock#47

Merged

3 tasks

edithturn mentioned this pull request Apr 19, 2022

Use inclusive words in Apache Airflow project #15994

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move Kubernetes modules out of contrib to match 2.0 layout #8904

Move Kubernetes modules out of contrib to match 2.0 layout #8904

dimberman commented May 18, 2020 •

edited

dimberman commented May 18, 2020

ashb left a comment

dimberman commented May 18, 2020

dimberman commented May 18, 2020 •

edited

kaxil commented May 19, 2020 •

edited

potiuk commented May 19, 2020 •

edited

dimberman commented May 19, 2020

kaxil left a comment

kaxil May 19, 2020

dimberman May 19, 2020

dimberman May 19, 2020

dimberman May 19, 2020

kaxil May 19, 2020

dimberman May 19, 2020

ashb May 19, 2020

dimberman May 19, 2020 •

edited

ashb May 19, 2020

ashb commented May 21, 2020

		'Uploading {} to wasb://{} '
		'as {}'.format(self.file_path, self.container_name, self.blob_name)

Move Kubernetes modules out of contrib to match 2.0 layout #8904

Move Kubernetes modules out of contrib to match 2.0 layout #8904

Conversation

dimberman commented May 18, 2020 • edited

dimberman commented May 18, 2020

ashb left a comment

Choose a reason for hiding this comment

dimberman commented May 18, 2020

dimberman commented May 18, 2020 • edited

kaxil commented May 19, 2020 • edited

potiuk commented May 19, 2020 • edited

dimberman commented May 19, 2020

kaxil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dimberman May 19, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashb commented May 21, 2020

dimberman commented May 18, 2020 •

edited

dimberman commented May 18, 2020 •

edited

kaxil commented May 19, 2020 •

edited

potiuk commented May 19, 2020 •

edited

dimberman May 19, 2020 •

edited