dstandish commented on a change in pull request #13247:
URL: https://github.com/apache/airflow/pull/13247#discussion_r570366452
##########
File path: airflow/cli/cli_parser.py
##########
@@ -58,9 +58,15 @@ class DefaultHelpParser(argparse.ArgumentParser):
def _check_value(self, action, value):
"""Override _check_value and check conditionally added command"""
executor = conf.get('core', 'EXECUTOR')
- if value == 'celery' and executor not in (CELERY_EXECUTOR,
CELERY_KUBERNETES_EXECUTOR):
- message = f'celery subcommand works only with CeleryExecutor, your
current executor: {executor}'
- raise ArgumentError(action, message)
+ if value == 'celery':
+ if executor != CELERY_EXECUTOR:
Review comment:
> we should at least take care of it when running "airflow celery
worker" (a less hacky --- so that users don't need to have a different
configuration for the Celery Worker.
another way to think about it @kaxil is, why does the celery worker even
need _any_ executor configured?
the celery worker does what the celery worker does. it shouldn't really
need to instantiate an executor at all i would think. shouldn't only the
scheduler need to do that?
so while presumably we can fix this fork issue by making changes to CKE like
you were doing in your local branch, another possible approach might be to make
it so that the celery worker command somehow doesn't create / use / invoke an
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:
[email protected]
