You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/03/27 11:02:15 UTC

[GitHub] [airflow] zeroam opened a new pull request #7902: Enable celery command in any environment

zeroam opened a new pull request #7902: Enable celery command in any environment
URL: https://github.com/apache/airflow/pull/7902
 
 
   Set celery command visible even if the executor is not CeleryExecutor
   Instead, raise AirflowException using celery command when the executor is
   not CeleryExecutor or backend is sqlite
   
   `airflow` command
   Before:
   ![image](https://user-images.githubusercontent.com/41822575/77749493-767af980-7065-11ea-986c-7b0b4caf8d23.png)
   
   After:
   ![image](https://user-images.githubusercontent.com/41822575/77749547-8b578d00-7065-11ea-8fda-743e28e69960.png)
   
   `airflow celery` command
   After (executor is SequentialExecutor):
   ![image](https://user-images.githubusercontent.com/41822575/77749711-d2458280-7065-11ea-931b-c0817ca3953f.png)
   
   
   reference from: #7873, #700
   reviewed from: @mik-laj
   
   ---
   Issue link: WILL BE INSERTED BY [boring-cyborg](https://github.com/kaxil/boring-cyborg)
   
   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)
   - [ ] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
   - [ ] Relevant documentation is updated including usage instructions.
   - [ ] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
   
   ---
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+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 [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
   Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] turbaszek commented on issue #7902: Enable celery command in any environment

Posted by GitBox <gi...@apache.org>.

turbaszek commented on issue #7902: Enable celery command in any environment
URL: https://github.com/apache/airflow/pull/7902#issuecomment-605511685
 
 
   Hi @zeroam Travis is sad due to some static checks. Please take a look at our pre-commits:
   https://github.com/apache/airflow/blob/master/STATIC_CODE_CHECKS.rst

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] turbaszek commented on issue #7902: Enable celery command in any environment

Posted by GitBox <gi...@apache.org>.

turbaszek commented on issue #7902: Enable celery command in any environment
URL: https://github.com/apache/airflow/pull/7902#issuecomment-605004203
 
 
   > > What would you say to write a nice message in case of airflow celery + LocalExecutor instead of raising an exception? I would feel better with "Hey this work only with celery, so no action was taken" than with stacktrace from exception :)
   > 
   > @turbaszek Thanks! How about this one?
   > ![image](https://user-images.githubusercontent.com/41822575/77759467-1c376400-7078-11ea-9739-98675f5e19eb.png)
   > This shows help message instead of Exception.
   
   Definitely better! What about "celery command works only with CeleryExecutor. Your current executor: ExecutorName for more information check the help above"
   
   Also, would you mind to extend current desription of `celery` sub command? For example:
   "Start celery components. Works only when using CeleryExecutor for more information see: here_link_to_docs"

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] zeroam commented on a change in pull request #7902: Enable celery command in any environment

Posted by GitBox <gi...@apache.org>.

zeroam commented on a change in pull request #7902: Enable celery command in any environment
URL: https://github.com/apache/airflow/pull/7902#discussion_r399731734
 
 

 ##########
 File path: airflow/cli/cli_parser.py
 ##########
 @@ -55,6 +55,24 @@ def command(*args, **kwargs):
     return command
 
 
+class DefaultHelpParser(argparse.ArgumentParser):
+    """CustomParser to display help message"""
+
+    def _check_value(self, action, value):
+        """Override _check_value and check conditionally added command"""
+        executor = conf.get('core', 'EXECUTOR')
+        if value == 'celery' and executor != ExecutorLoader.CELERY_EXECUTOR:
+            message = f'celery command works only with CeleryExecutor, your current executor: {executor}'
 
 Review comment:
   Good point. Thanks

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #7902: Enable celery command in any environment

Posted by GitBox <gi...@apache.org>.

codecov-io edited a comment on issue #7902: Enable celery command in any environment
URL: https://github.com/apache/airflow/pull/7902#issuecomment-605728711
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=h1) Report
   > Merging [#7902](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/8456b30c412d95f7d4fa6e2af655c38d31ecead3&el=desc) will **decrease** coverage by `0.63%`.
   > The diff coverage is `63.63%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7902/graphs/tree.svg?width=650&height=150&src=pr&token=WdLKlKHOAU)](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #7902      +/-   ##
   ==========================================
   - Coverage   87.16%   86.52%   -0.64%     
   ==========================================
     Files         931      932       +1     
     Lines       45155    45191      +36     
   ==========================================
   - Hits        39358    39101     -257     
   - Misses       5797     6090     +293     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/cli/cli\_parser.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9jbGkvY2xpX3BhcnNlci5weQ==) | `97.24% <63.63%> (-0.89%)` | :arrow_down: |
   | [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.25% <0.00%> (-72.50%)` | :arrow_down: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
   | [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `47.82% <0.00%> (-52.18%)` | :arrow_down: |
   | [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
   | [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0.00%> (-46.67%)` | :arrow_down: |
   | [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0.00%> (-45.08%)` | :arrow_down: |
   | [airflow/providers/redis/sensors/redis\_key.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvc2Vuc29ycy9yZWRpc19rZXkucHk=) | `61.53% <0.00%> (-38.47%)` | :arrow_down: |
   | [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `50.67% <0.00%> (-37.84%)` | :arrow_down: |
   | ... and [10 more](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=footer). Last update [8456b30...b56941d](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] zhongjiajie commented on a change in pull request #7902: Enable celery command in any environment

Posted by GitBox <gi...@apache.org>.

zhongjiajie commented on a change in pull request #7902: Enable celery command in any environment
URL: https://github.com/apache/airflow/pull/7902#discussion_r399675719
 
 

 ##########
 File path: airflow/cli/cli_parser.py
 ##########
 @@ -1102,11 +1120,12 @@ class GroupCommand(NamedTuple):
         func=lazy_load_command('airflow.cli.commands.config_command.show_config'),
         args=(),
     ),
-]
-if conf.get("core", "EXECUTOR") == ExecutorLoader.CELERY_EXECUTOR or BUILD_DOCS:
-    airflow_commands.append(GroupCommand(
+    GroupCommand(
         name="celery",
-        help="Start celery components",
+        help=(
+            'Start celery components. Works only when using CeleryExecutor. for more information, see '
 
 Review comment:
   ```suggestion
               'Start celery components. Works only when using CeleryExecutor. For more information, see '
   ```

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] zeroam commented on issue #7902: Enable celery command in any environment

Posted by GitBox <gi...@apache.org>.

zeroam commented on issue #7902: Enable celery command in any environment
URL: https://github.com/apache/airflow/pull/7902#issuecomment-605037589
 
 
   @turbaszek your suggestion is more clear! I've changed help message and changed `celery` subcommand help message

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] zhongjiajie commented on issue #7902: Enable celery command in any environment

Posted by GitBox <gi...@apache.org>.

zhongjiajie commented on issue #7902: Enable celery command in any environment
URL: https://github.com/apache/airflow/pull/7902#issuecomment-605464224
 
 
   BTW, code LGTM

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] turbaszek commented on a change in pull request #7902: Enable celery command in any environment

Posted by GitBox <gi...@apache.org>.

turbaszek commented on a change in pull request #7902: Enable celery command in any environment
URL: https://github.com/apache/airflow/pull/7902#discussion_r399641898
 
 

 ##########
 File path: airflow/cli/cli_parser.py
 ##########
 @@ -55,6 +55,24 @@ def command(*args, **kwargs):
     return command
 
 
+class DefaultHelpParser(argparse.ArgumentParser):
+    """CustomParser to display help message"""
+
+    def _check_value(self, action, value):
+        """Override _check_value and check conditionally added command"""
+        executor = conf.get('core', 'EXECUTOR')
+        if value == 'celery' and executor != ExecutorLoader.CELERY_EXECUTOR:
+            message = f'celery command works only with CeleryExecutor, your current executor: {executor}'
+            raise ArgumentError(action, message)
+
+        super()._check_value(action, value)
+
+    def error(self, message):
+        """Override error and use print_instead of print_usage"""
+        self.print_help()
+        self.exit(2, '\n{} command error: {}, see help above.\n'.format(self.prog, message))
 
 Review comment:
   Should we use f-string? 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] zeroam commented on a change in pull request #7902: Enable celery command in any environment

Posted by GitBox <gi...@apache.org>.

zeroam commented on a change in pull request #7902: Enable celery command in any environment
URL: https://github.com/apache/airflow/pull/7902#discussion_r399649960
 
 

 ##########
 File path: airflow/cli/cli_parser.py
 ##########
 @@ -55,6 +55,24 @@ def command(*args, **kwargs):
     return command
 
 
+class DefaultHelpParser(argparse.ArgumentParser):
+    """CustomParser to display help message"""
+
+    def _check_value(self, action, value):
+        """Override _check_value and check conditionally added command"""
+        executor = conf.get('core', 'EXECUTOR')
+        if value == 'celery' and executor != ExecutorLoader.CELERY_EXECUTOR:
+            message = f'celery command works only with CeleryExecutor, your current executor: {executor}'
+            raise ArgumentError(action, message)
+
+        super()._check_value(action, value)
+
+    def error(self, message):
+        """Override error and use print_instead of print_usage"""
+        self.print_help()
+        self.exit(2, '\n{} command error: {}, see help above.\n'.format(self.prog, message))
 
 Review comment:
   Okay, I've changed 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] turbaszek commented on issue #7902: Enable celery command in any environment

Posted by GitBox <gi...@apache.org>.

turbaszek commented on issue #7902: Enable celery command in any environment
URL: https://github.com/apache/airflow/pull/7902#issuecomment-604985038
 
 
   What would you say to write a nice message in case of `airflow celery` + LocalExecutor instead of raising an exception? I would feel better with "Hey this work only with celery, so no action was taken" than with stacktrace from exception :)

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #7902: Enable celery command in any environment

Posted by GitBox <gi...@apache.org>.

codecov-io edited a comment on issue #7902: Enable celery command in any environment
URL: https://github.com/apache/airflow/pull/7902#issuecomment-605728711
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=h1) Report
   > Merging [#7902](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/8456b30c412d95f7d4fa6e2af655c38d31ecead3&el=desc) will **decrease** coverage by `0.63%`.
   > The diff coverage is `63.63%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7902/graphs/tree.svg?width=650&height=150&src=pr&token=WdLKlKHOAU)](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #7902      +/-   ##
   ==========================================
   - Coverage   87.16%   86.52%   -0.64%     
   ==========================================
     Files         931      932       +1     
     Lines       45155    45191      +36     
   ==========================================
   - Hits        39358    39101     -257     
   - Misses       5797     6090     +293     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/cli/cli\_parser.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9jbGkvY2xpX3BhcnNlci5weQ==) | `97.24% <63.63%> (-0.89%)` | :arrow_down: |
   | [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.25% <0.00%> (-72.50%)` | :arrow_down: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
   | [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `47.82% <0.00%> (-52.18%)` | :arrow_down: |
   | [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
   | [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0.00%> (-46.67%)` | :arrow_down: |
   | [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0.00%> (-45.08%)` | :arrow_down: |
   | [airflow/providers/redis/sensors/redis\_key.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvc2Vuc29ycy9yZWRpc19rZXkucHk=) | `61.53% <0.00%> (-38.47%)` | :arrow_down: |
   | [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `50.67% <0.00%> (-37.84%)` | :arrow_down: |
   | ... and [10 more](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=footer). Last update [8456b30...b56941d](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] potiuk merged pull request #7902: Enable celery command in any environment

Posted by GitBox <gi...@apache.org>.

potiuk merged pull request #7902: Enable celery command in any environment
URL: https://github.com/apache/airflow/pull/7902
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] zhongjiajie commented on a change in pull request #7902: Enable celery command in any environment

Posted by GitBox <gi...@apache.org>.

zhongjiajie commented on a change in pull request #7902: Enable celery command in any environment
URL: https://github.com/apache/airflow/pull/7902#discussion_r399675764
 
 

 ##########
 File path: airflow/cli/cli_parser.py
 ##########
 @@ -55,6 +55,24 @@ def command(*args, **kwargs):
     return command
 
 
+class DefaultHelpParser(argparse.ArgumentParser):
+    """CustomParser to display help message"""
+
+    def _check_value(self, action, value):
+        """Override _check_value and check conditionally added command"""
+        executor = conf.get('core', 'EXECUTOR')
+        if value == 'celery' and executor != ExecutorLoader.CELERY_EXECUTOR:
+            message = f'celery command works only with CeleryExecutor, your current executor: {executor}'
 
 Review comment:
   ```suggestion
               message = f'celery subcommand works only with CeleryExecutor, your current executor: {executor}'
   ```

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] zeroam commented on issue #7902: Enable celery command in any environment

Posted by GitBox <gi...@apache.org>.

zeroam commented on issue #7902: Enable celery command in any environment
URL: https://github.com/apache/airflow/pull/7902#issuecomment-604994444
 
 
   > What would you say to write a nice message in case of airflow celery + LocalExecutor instead of raising an exception? I would feel better with "Hey this work only with celery, so no action was taken" than with stacktrace from exception :)
   
   @turbaszek Thanks! How about this one?
   ![image](https://user-images.githubusercontent.com/41822575/77759467-1c376400-7078-11ea-9739-98675f5e19eb.png)
   This shows help message instead of Exception.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io commented on issue #7902: Enable celery command in any environment

Posted by GitBox <gi...@apache.org>.

codecov-io commented on issue #7902: Enable celery command in any environment
URL: https://github.com/apache/airflow/pull/7902#issuecomment-605728711
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=h1) Report
   > Merging [#7902](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/8456b30c412d95f7d4fa6e2af655c38d31ecead3&el=desc) will **decrease** coverage by `0.63%`.
   > The diff coverage is `63.63%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7902/graphs/tree.svg?width=650&height=150&src=pr&token=WdLKlKHOAU)](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #7902      +/-   ##
   ==========================================
   - Coverage   87.16%   86.52%   -0.64%     
   ==========================================
     Files         931      932       +1     
     Lines       45155    45191      +36     
   ==========================================
   - Hits        39358    39101     -257     
   - Misses       5797     6090     +293     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/cli/cli\_parser.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9jbGkvY2xpX3BhcnNlci5weQ==) | `97.24% <63.63%> (-0.89%)` | :arrow_down: |
   | [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.25% <0.00%> (-72.50%)` | :arrow_down: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
   | [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `47.82% <0.00%> (-52.18%)` | :arrow_down: |
   | [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
   | [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0.00%> (-46.67%)` | :arrow_down: |
   | [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0.00%> (-45.08%)` | :arrow_down: |
   | [airflow/providers/redis/sensors/redis\_key.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvc2Vuc29ycy9yZWRpc19rZXkucHk=) | `61.53% <0.00%> (-38.47%)` | :arrow_down: |
   | [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `50.67% <0.00%> (-37.84%)` | :arrow_down: |
   | ... and [10 more](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=footer). Last update [8456b30...b56941d](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] codecov-io edited a comment on issue #7902: Enable celery command in any environment

Posted by GitBox <gi...@apache.org>.

codecov-io edited a comment on issue #7902: Enable celery command in any environment
URL: https://github.com/apache/airflow/pull/7902#issuecomment-605728711
 
 
   # [Codecov](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=h1) Report
   > Merging [#7902](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/8456b30c412d95f7d4fa6e2af655c38d31ecead3&el=desc) will **decrease** coverage by `0.63%`.
   > The diff coverage is `63.63%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7902/graphs/tree.svg?width=650&height=150&src=pr&token=WdLKlKHOAU)](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #7902      +/-   ##
   ==========================================
   - Coverage   87.16%   86.52%   -0.64%     
   ==========================================
     Files         931      932       +1     
     Lines       45155    45191      +36     
   ==========================================
   - Hits        39358    39101     -257     
   - Misses       5797     6090     +293     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [airflow/cli/cli\_parser.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9jbGkvY2xpX3BhcnNlci5weQ==) | `97.24% <63.63%> (-0.89%)` | :arrow_down: |
   | [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.25% <0.00%> (-72.50%)` | :arrow_down: |
   | [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
   | [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `47.82% <0.00%> (-52.18%)` | :arrow_down: |
   | [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
   | [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0.00%> (-46.67%)` | :arrow_down: |
   | [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0.00%> (-45.08%)` | :arrow_down: |
   | [airflow/providers/redis/sensors/redis\_key.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvc2Vuc29ycy9yZWRpc19rZXkucHk=) | `61.53% <0.00%> (-38.47%)` | :arrow_down: |
   | [airflow/executors/celery\_executor.py](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGVjdXRvcnMvY2VsZXJ5X2V4ZWN1dG9yLnB5) | `50.67% <0.00%> (-37.84%)` | :arrow_down: |
   | ... and [10 more](https://codecov.io/gh/apache/airflow/pull/7902/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=footer). Last update [8456b30...b56941d](https://codecov.io/gh/apache/airflow/pull/7902?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] zeroam commented on issue #7902: Enable celery command in any environment

Posted by GitBox <gi...@apache.org>.

zeroam commented on issue #7902: Enable celery command in any environment
URL: https://github.com/apache/airflow/pull/7902#issuecomment-605598316
 
 
   @turbaszek Thanks for the advice!
   I checked pre-commit and passed the static test.
   but I failed in test_celery_command
   ```
   FAILED tests/cli/commands/test_celery_command.py::TestCeleryStopCommand::test_if_right_pid_is_read - SystemExit: 2
   FAILED tests/cli/commands/test_celery_command.py::TestCeleryStopCommand::test_same_pid_file_is_used_in_start_and_stop - SystemExit: 2
   FAILED tests/cli/commands/test_celery_command.py::TestWorkerStart::test_worker_started_with_required_arguments - SystemExit: 2
   ```
   I will fix 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] turbaszek edited a comment on issue #7902: Enable celery command in any environment

Posted by GitBox <gi...@apache.org>.

turbaszek edited a comment on issue #7902: Enable celery command in any environment
URL: https://github.com/apache/airflow/pull/7902#issuecomment-605004203
 
 
   > > What would you say to write a nice message in case of airflow celery + LocalExecutor instead of raising an exception? I would feel better with "Hey this work only with celery, so no action was taken" than with stacktrace from exception :)
   > 
   > @turbaszek Thanks! How about this one?
   > ![image](https://user-images.githubusercontent.com/41822575/77759467-1c376400-7078-11ea-9739-98675f5e19eb.png)
   > This shows help message instead of Exception.
   
   Definitely better! What about "celery command works only with CeleryExecutor, your current executor: ExecutorName. For more information check the help above"
   
   Also, would you mind to extend current desription of `celery` sub command? For example:
   "Start celery components. Works only when using CeleryExecutor for more information see: here_link_to_docs"

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services