Skip to content

Add documentation generation for CLI commands from executors - #33081

Merged
potiuk merged 1 commit into
apache:mainfrom
potiuk:add-cli-executor-command-docs
Aug 4, 2023
Merged

Add documentation generation for CLI commands from executors#33081
potiuk merged 1 commit into
apache:mainfrom
potiuk:add-cli-executor-command-docs

Conversation

@potiuk

@potiukpotiuk commented Aug 3, 2023•
edited
Loading

Copy link
Copy Markdown
Member

The #29055 moved relevant CLI command definition to executors but it automatically removed the command from generated documentation.

Closes: #27932

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@potiuk
potiukforce-pushed the add-cli-executor-command-docs branch from f6ae439 to 7d581ffCompareAugust 3, 2023 18:52
@potiuk

potiuk commented Aug 3, 2023

Copy link
Copy Markdown
MemberAuthor

Example Celery docs:

image

@potiuk

potiuk commented Aug 3, 2023

Copy link
Copy Markdown
MemberAuthor

I will need to add a second one to add links to the providers from Airflow docs - it will have to be done after this one is merged though due to cyclic celery -> airflow -> kubernetes -> celery dependency and I have to make sure the inventory for the Celery / Kubernetes providers is pushed to S3 before the Airflow one might succed.

o-nikolas
o-nikolas reviewed Aug 3, 2023
View reviewed changes
Comment threadairflow/providers/cncf/kubernetes/executors/kubernetes_executor.py Outdated
@potiuk
potiukforce-pushed the add-cli-executor-command-docs branch from 7d581ff to c44ae7bCompareAugust 3, 2023 20:00
@potiuk
potiukforce-pushed the add-cli-executor-command-docs branch from c44ae7b to 3e5b76eCompareAugust 4, 2023 10:38
@potiuk
aDd documentation generation for CLI commands from executors
3d3f80a
The apache#29055 moved relevant CLI command definition to executors but
it automatically removed the command from generated documentation.
This PR brings it back for both Celery and Cncf Kubernetes provider
Closes: apache#27932
@potiuk
potiukforce-pushed the add-cli-executor-command-docs branch from 3e5b76e to 3d3f80aCompareAugust 4, 2023 12:17
@potiuk

potiuk commented Aug 4, 2023

Copy link
Copy Markdown
MemberAuthor

That one should be ready - for now it has only "providers" side - I will add separately the "airflow" side to link to the documentation of the CLI commands in providers cc: @eladkal

@potiuk

potiuk commented Aug 4, 2023

Copy link
Copy Markdown
MemberAuthor

All green !

eladkal
eladkal approved these changes Aug 4, 2023
View reviewed changes

@eladkaleladkal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@eladkaleladkal added this to the Airflow 2.7.0 milestone Aug 4, 2023
@potiuk
potiuk merged commit 879fd34 into apache:mainAug 4, 2023
@potiuk
potiuk deleted the add-cli-executor-command-docs branch August 4, 2023 17:26
@eladkaleladkal mentioned this pull request Aug 4, 2023
Closed
44 tasks
o-nikolas
o-nikolas reviewed Aug 4, 2023
View reviewed changes
Comment threadairflow/providers/celery/executors/celery_executor.py

:meta private:
"""
return CeleryExecutor._get_parser()

@o-nikolaso-nikolasAug 4, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Very nice now abstracted away to the base class!

potiuk added a commit to potiuk/airflow that referenced this pull request Aug 5, 2023
@potiuk
Split PROD image verification to a separate step in CI
edd06c7
The PROD image verification happens after the images are pulled
in the "wait for PROD images" step. This verification is pretty
helpful in detecting cases where there are some "installed airflow"
problems (for example recently it helped to avoid a circular import
problem in apache#33081 as one of the tests failed when images were
verified. However PROD image wait might fail for other reasons and
sometimes might be "neglected" as temporary failure.
Separating verification will allow to clearly surface that the
problem is with verification, not pulling the images.
@potiukpotiuk mentioned this pull request Aug 5, 2023
Merged
potiuk added a commit that referenced this pull request Aug 5, 2023
@potiuk
Split PROD image verification to a separate step in CI (#33140)
c310703
The PROD image verification happens after the images are pulled
in the "wait for PROD images" step. This verification is pretty
helpful in detecting cases where there are some "installed airflow"
problems (for example recently it helped to avoid a circular import
problem in #33081 as one of the tests failed when images were
verified. However PROD image wait might fail for other reasons and
sometimes might be "neglected" as temporary failure.
Separating verification will allow to clearly surface that the
problem is with verification, not pulling the images.
potiuk added a commit to potiuk/airflow that referenced this pull request Aug 5, 2023
@potiuk
Add links to Celery and Kubernetes executor CLI commands to Airflow
157c01b
The apache#33081 added documentation generation for CLI commands contributed
by executors coming from providers. Adding Airflow documentation in
the same PR was not really possible because it created a documentation
reference circle and we had to wait until the documentation inventory
has been released in S3 and since this already happened, we can now
add links to provider CLIs from Airflow.
@potiukpotiuk mentioned this pull request Aug 5, 2023
Merged
potiuk added a commit that referenced this pull request Aug 5, 2023
@potiuk
Add links to Celery and Kubernetes executor CLI commands to Airflow (#…
9910d92
…33143)
The #33081 added documentation generation for CLI commands contributed
by executors coming from providers. Adding Airflow documentation in
the same PR was not really possible because it created a documentation
reference circle and we had to wait until the documentation inventory
has been released in S3 and since this already happened, we can now
add links to provider CLIs from Airflow.
@ephraimbuddyephraimbuddy added the type:improvement Changelog: Improvements label Aug 8, 2023
ephraimbuddy pushed a commit that referenced this pull request Aug 8, 2023
@potiuk@ephraimbuddy
Add links to Celery and Kubernetes executor CLI commands to Airflow (#…
41d57f0
…33143)
The #33081 added documentation generation for CLI commands contributed
by executors coming from providers. Adding Airflow documentation in
the same PR was not really possible because it created a documentation
reference circle and we had to wait until the documentation inventory
has been released in S3 and since this already happened, we can now
add links to provider CLIs from Airflow.
(cherry picked from commit 9910d92)
ephraimbuddy pushed a commit that referenced this pull request Aug 8, 2023
@potiuk@ephraimbuddy
aDd documentation generation for CLI commands from executors (#33081)
1109ea9
The #29055 moved relevant CLI command definition to executors but
it automatically removed the command from generated documentation.
This PR brings it back for both Celery and Cncf Kubernetes provider
Closes: #27932
(cherry picked from commit 879fd34)
ephraimbuddy pushed a commit that referenced this pull request Aug 8, 2023
@potiuk@ephraimbuddy
Split PROD image verification to a separate step in CI (#33140)
86f1dc1
The PROD image verification happens after the images are pulled
in the "wait for PROD images" step. This verification is pretty
helpful in detecting cases where there are some "installed airflow"
problems (for example recently it helped to avoid a circular import
problem in #33081 as one of the tests failed when images were
verified. However PROD image wait might fail for other reasons and
sometimes might be "neglected" as temporary failure.
Separating verification will allow to clearly surface that the
problem is with verification, not pulling the images.
(cherry picked from commit c310703)
@ephraimbuddyephraimbuddy added AIP-51 AIP-51: Remove executor coupling from Core changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) and removed type:improvement Changelog: Improvements labels Aug 10, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@o-nikolaso-nikolaso-nikolas left review comments
@eladkaleladkaleladkal approved these changes
@jedcunninghamjedcunninghamAwaiting requested review from jedcunningham
@ephraimbuddyephraimbuddyAwaiting requested review from ephraimbuddy
@pierrejeambrunpierrejeambrunAwaiting requested review from pierrejeambrun
@kaxilkaxilAwaiting requested review from kaxil
@XD-DENGXD-DENGAwaiting requested review from XD-DENG
@ashbashbAwaiting requested review from ashb

Assignees

No one assigned

Labels

AIP-51AIP-51: Remove executor coupling from Corearea:CLIarea:providerschangelog:skipChanges that should be skipped from the changelog (CI, tests, etc..)kind:documentationprovider:celeryprovider:cncf-kubernetesKubernetes (k8s) provider related issues

Projects

No open projects
AIP-51 Removing Executor Coupling from Core Airflow
Status: ✅ Done

Milestone

Airflow 2.7.0

Development

Successfully merging this pull request may close these issues.

AIP-51 - Executor Specific CLI Commands

4 participants

@potiuk@eladkal@o-nikolas@ephraimbuddy