You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "potiuk (via GitHub)" <gi...@apache.org> on 2023/08/05 17:27:46 UTC

[GitHub] [airflow] potiuk opened a new pull request, #33143: Add links to Celery and Kubernetes executor CLI commands to Airflow

potiuk opened a new pull request, #33143:
URL: https://github.com/apache/airflow/pull/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.
   
   <!--
    Licensed to the Apache Software Foundation (ASF) under one
    or more contributor license agreements.  See the NOTICE file
    distributed with this work for additional information
    regarding copyright ownership.  The ASF licenses this file
    to you under the Apache License, Version 2.0 (the
    "License"); you may not use this file except in compliance
    with the License.  You may obtain a copy of the License at
   
      http://www.apache.org/licenses/LICENSE-2.0
   
    Unless required by applicable law or agreed to in writing,
    software distributed under the License is distributed on an
    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    KIND, either express or implied.  See the License for the
    specific language governing permissions and limitations
    under the License.
    -->
   
   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   
   Feel free to ping committers for the review!
   
   In case of an existing issue, reference it using one of the following:
   
   closes: #ISSUE
   related: #ISSUE
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   
   
   <!-- Please keep an empty line above the dashes. -->
   ---
   **^ Add meaningful description above**
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   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](https://github.com/apache/airflow/tree/main/newsfragments).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] potiuk commented on a diff in pull request #33143: Add links to Celery and Kubernetes executor CLI commands to Airflow

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #33143:
URL: https://github.com/apache/airflow/pull/33143#discussion_r1287690751


##########
docs/apache-airflow/cli-and-env-variables-ref.rst:
##########
@@ -34,6 +34,12 @@ development and testing.
     :local:
     :depth: 2
 
+Providers that implement executors might contribute additional commands to the CLI. Here are the commands
+contributed by the community providers:

Review Comment:
   Yes.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] eladkal commented on a diff in pull request #33143: Add links to Celery and Kubernetes executor CLI commands to Airflow

Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #33143:
URL: https://github.com/apache/airflow/pull/33143#discussion_r1285110456


##########
docs/apache-airflow/cli-and-env-variables-ref.rst:
##########
@@ -34,6 +34,12 @@ development and testing.
     :local:
     :depth: 2
 
+Providers that implement executors might contribute additional commands to the CLI. Here are the commands
+contributed by the community providers:
+
+* Celery Executor and related CLI commands: :doc:`apache-airflow-providers-celery:cli-ref`
+* Kubernetes Executor and related CLI commands: :doc:`apache-airflow-providers-cncf-kubernetes:cli-ref`
+

Review Comment:
   Do we want this list to be auto generated/discovered like we do with Notifiers?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] potiuk merged pull request #33143: Add links to Celery and Kubernetes executor CLI commands to Airflow

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #33143:
URL: https://github.com/apache/airflow/pull/33143


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] potiuk commented on pull request #33143: Add links to Celery and Kubernetes executor CLI commands to Airflow

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #33143:
URL: https://github.com/apache/airflow/pull/33143#issuecomment-1666559968

   Looks like that:
   
   <img width="1597" alt="Screenshot 2023-08-05 at 19 21 04" src="https://github.com/apache/airflow/assets/595491/bfe44a2b-b24e-4d73-90bf-fd31b00337b2">
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] potiuk commented on a diff in pull request #33143: Add links to Celery and Kubernetes executor CLI commands to Airflow

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #33143:
URL: https://github.com/apache/airflow/pull/33143#discussion_r1285115308


##########
docs/apache-airflow/cli-and-env-variables-ref.rst:
##########
@@ -34,6 +34,12 @@ development and testing.
     :local:
     :depth: 2
 
+Providers that implement executors might contribute additional commands to the CLI. Here are the commands
+contributed by the community providers:
+
+* Celery Executor and related CLI commands: :doc:`apache-airflow-providers-celery:cli-ref`
+* Kubernetes Executor and related CLI commands: :doc:`apache-airflow-providers-cncf-kubernetes:cli-ref`
+

Review Comment:
   I thought about it and I decided not to - maybe even forever.
   
   Not for now - that's not as straightforward - also because the CLI commands can be technically added by different executors (LocalKubernetesExecutor has the same commands as KubernetesExecutor and CeleryKubernetesExecutor adds both Celery and Kubernetes one) - so we would have to somehow de-duplicate them - and it's not obvious how to fully automate that until we have more executors that are potentially extending those we already have.
   
   Since those are only "executors" that can add the CLI commands, they will be added rarely enough so that we do not have to automatically discover those, I think.
   
   But if we see the need in the future we can always add it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] o-nikolas commented on a diff in pull request #33143: Add links to Celery and Kubernetes executor CLI commands to Airflow

Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on code in PR #33143:
URL: https://github.com/apache/airflow/pull/33143#discussion_r1287684917


##########
docs/apache-airflow/cli-and-env-variables-ref.rst:
##########
@@ -34,6 +34,12 @@ development and testing.
     :local:
     :depth: 2
 
+Providers that implement executors might contribute additional commands to the CLI. Here are the commands
+contributed by the community providers:

Review Comment:
   Might be worth it to add a Note that says something like "NOTE: only commands for the currently configured executor will be exposed on the CLI" 
   
   Just so that behaviour is documented, WDYT? @potiuk
   
   I can open a PR for that as well.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] eladkal commented on a diff in pull request #33143: Add links to Celery and Kubernetes executor CLI commands to Airflow

Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #33143:
URL: https://github.com/apache/airflow/pull/33143#discussion_r1285110456


##########
docs/apache-airflow/cli-and-env-variables-ref.rst:
##########
@@ -34,6 +34,12 @@ development and testing.
     :local:
     :depth: 2
 
+Providers that implement executors might contribute additional commands to the CLI. Here are the commands
+contributed by the community providers:
+
+* Celery Executor and related CLI commands: :doc:`apache-airflow-providers-celery:cli-ref`
+* Kubernetes Executor and related CLI commands: :doc:`apache-airflow-providers-cncf-kubernetes:cli-ref`
+

Review Comment:
   Do we want this list to be auto generated/discovered like we do with Notifiers? (maybe a followup issue to describe this task so someone can pick it up?)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org