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/09/01 15:17:45 UTC
[GitHub] [airflow] dimberman opened a new pull request #10677: Add dry-run mode to the KubernetesExecutor
dimberman opened a new pull request #10677:
URL: https://github.com/apache/airflow/pull/10677
To aide in migration testing, and testing in general
we add a dry-run mode that allows the KubernetesExecutor to
write all pods into yaml files instead of of launching them.
<!--
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 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/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
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).
----------------------------------------------------------------
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
[GitHub] [airflow] dimberman commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483259731
##########
File path: airflow/cli/cli_parser.py
##########
@@ -142,6 +142,10 @@ def positive_int(value):
("-e", "--end-date"),
help="Override end_date YYYY-MM-DD",
type=parsedate)
+ARG_OUTPUT_PATH = Arg(
+ ("-o", "--output-path",),
+ help="The output for generated yaml files",
+ default="/tmp/airflow_yaml_output/")
Review comment:
By putting it in /tmp we can assure that it will eventually be deleted as the system needs space.
----------------------------------------------------------------
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
[GitHub] [airflow] dimberman commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482191652
##########
File path: tests/cli/commands/test_dag_command.py
##########
@@ -139,6 +139,12 @@ def test_show_dag_print(self):
self.assertIn("graph [label=example_bash_operator labelloc=t rankdir=LR]", out)
self.assertIn("runme_2 -> run_after_loop", out)
+ def test_generate_dag_yaml(self):
+ with contextlib.redirect_stdout(io.StringIO()) as temp_stdout:
+ dag_command.generate_pod_yaml(self.parser.parse_args([
+ 'dags', 'generate_yaml', '--dag-id', 'example_bash_operator']))
+
Review comment:
@mik-laj yes I plan to add assertions that a yaml file is created and that it's a valid pod.
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482190501
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -378,6 +379,47 @@ def dag_list_dag_runs(args, dag=None):
print(table)
+@provide_session
+@cli_utils.action_logging
+def generate_pod_yaml(args, session=None):
+ """Generates yaml files for each task in the DAG. Used for testing output of KubernetesExecutor"""
+
+ from airflow.executors.kubernetes_executor import AirflowKubernetesScheduler, KubeConfig
+ from airflow.kubernetes import pod_generator
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.kubernetes.worker_configuration import WorkerConfiguration
+ execution_date = datetime.datetime(2020,11,3)
+ dag = get_dag(subdir=args.subdir, dag_id=args.dag_id)
+ yaml_output_path = args.output_path or "/tmp/airflow_generated_yaml/"
Review comment:
Should be `sys.stdout` by default?
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483145506
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -378,6 +380,48 @@ def dag_list_dag_runs(args, dag=None):
print(table)
+@cli_utils.action_logging
+def generate_pod_yaml(args):
+ """Generates yaml files for each task in the DAG. Used for testing output of KubernetesExecutor"""
+
+ from airflow.executors.kubernetes_executor import AirflowKubernetesScheduler, KubeConfig
+ from airflow.kubernetes import pod_generator
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.kubernetes.worker_configuration import WorkerConfiguration
+ from airflow.settings import pod_mutation_hook
+ from airflow.utils import timezone
+ execution_date = args.exec_date or timezone.utcnow()
+ dag = get_dag(subdir=args.subdir, dag_id=args.dag_id)
+ yaml_output_path = args.output_path or "/tmp/airflow_generated_yaml/"
+ kube_config = KubeConfig()
+ for task in dag.tasks:
+ ti = TaskInstance(task, execution_date)
+ pod = PodGenerator.construct_pod(
+ dag_id=args.dag_id,
+ task_id=ti.task_id,
+ pod_id=AirflowKubernetesScheduler._create_pod_id( # pylint: disable=W0212
+ args.dag_id, ti.task_id),
+ try_number=ti.try_number,
+ date=ti.execution_date,
+ command=ti.command_as_list(),
+ kube_executor_config=PodGenerator.from_obj(ti.executor_config),
+ worker_uuid="worker-config",
+ namespace=kube_config.executor_namespace,
+ worker_config=WorkerConfiguration(kube_config=kube_config).as_pod()
+ )
+ pod_mutation_hook(pod)
+
+ from kubernetes.client.api_client import ApiClient
Review comment:
It can be at Line 392, it will still be inside the function
----------------------------------------------------------------
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
[GitHub] [airflow] dimberman commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483290900
##########
File path: airflow/cli/cli_parser.py
##########
@@ -142,6 +142,10 @@ def positive_int(value):
("-e", "--end-date"),
help="Override end_date YYYY-MM-DD",
type=parsedate)
+ARG_OUTPUT_PATH = Arg(
+ ("-o", "--output-path",),
+ help="The output for generated yaml files",
+ default="/tmp/airflow_yaml_output/")
Review comment:
@kaxil WDYT?
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483298651
##########
File path: airflow/cli/cli_parser.py
##########
@@ -66,6 +66,17 @@ def _check_value(self, action, value):
if value == 'celery' and executor != ExecutorLoader.CELERY_EXECUTOR:
message = f'celery subcommand works only with CeleryExecutor, your current executor: {executor}'
raise ArgumentError(action, message)
+ if value == 'kubernetes':
+ try:
+ from kubernetes.client import models
+ if not models:
+ message = 'kubernetes subcommand requires that ' \
Review comment:
Can you add full pip install command - ``pip install 'apache-airflow[cncf.kubernetes]' ``?
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482607325
##########
File path: tests/cli/commands/test_dag_command.py
##########
@@ -139,6 +140,19 @@ def test_show_dag_print(self):
self.assertIn("graph [label=example_bash_operator labelloc=t rankdir=LR]", out)
self.assertIn("runme_2 -> run_after_loop", out)
+ def test_generate_dag_yaml(self):
+
+ directory = "/tmp/airflow_dry_run_test/"
+ file_name = "example_bash_operator_run_after_loop_2020-11-03T00_00_00.yml"
+ if os.path.exists(directory):
+ shutil.rmtree(directory)
+ dag_command.generate_pod_yaml(self.parser.parse_args([
+ 'dags', 'generate_kubernetes_pod_yaml',
+ "--output-path", directory, 'example_bash_operator']))
+ self.assertEqual(len(os.listdir(directory)), 6)
+ self.assertTrue(os.path.isfile(directory + file_name))
+ self.assertNotEqual(os.stat(directory + file_name).st_size, 0)
Review comment:
you can use `assertGreater` here https://docs.python.org/3/library/unittest.html#unittest.TestCase.assertGreater
----------------------------------------------------------------
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
[GitHub] [airflow] dimberman commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483143457
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -378,6 +380,48 @@ def dag_list_dag_runs(args, dag=None):
print(table)
+@cli_utils.action_logging
+def generate_pod_yaml(args):
+ """Generates yaml files for each task in the DAG. Used for testing output of KubernetesExecutor"""
+
+ from airflow.executors.kubernetes_executor import AirflowKubernetesScheduler, KubeConfig
+ from airflow.kubernetes import pod_generator
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.kubernetes.worker_configuration import WorkerConfiguration
+ from airflow.settings import pod_mutation_hook
+ from airflow.utils import timezone
+ execution_date = args.exec_date or timezone.utcnow()
+ dag = get_dag(subdir=args.subdir, dag_id=args.dag_id)
+ yaml_output_path = args.output_path or "/tmp/airflow_generated_yaml/"
+ kube_config = KubeConfig()
+ for task in dag.tasks:
+ ti = TaskInstance(task, execution_date)
+ pod = PodGenerator.construct_pod(
+ dag_id=args.dag_id,
+ task_id=ti.task_id,
+ pod_id=AirflowKubernetesScheduler._create_pod_id( # pylint: disable=W0212
+ args.dag_id, ti.task_id),
+ try_number=ti.try_number,
+ date=ti.execution_date,
+ command=ti.command_as_list(),
+ kube_executor_config=PodGenerator.from_obj(ti.executor_config),
+ worker_uuid="worker-config",
+ namespace=kube_config.executor_namespace,
+ worker_config=WorkerConfiguration(kube_config=kube_config).as_pod()
+ )
+ pod_mutation_hook(pod)
+
+ from kubernetes.client.api_client import ApiClient
Review comment:
@kaxil this one has a reason: to avoid causing errors if a user does not have the kubernetes client installed.
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482203423
##########
File path: airflow/cli/cli_parser.py
##########
@@ -778,6 +783,12 @@ class GroupCommand(NamedTuple):
func=lazy_load_command('airflow.cli.commands.dag_command.dag_list_dags'),
args=(ARG_SUBDIR, ARG_OUTPUT),
),
+ ActionCommand(
+ name='generate_yaml',
+ help="Generate YAML files for all tasks in DAG",
+ func=lazy_load_command('airflow.cli.commands.dag_command.generate_pod_yaml'),
+ args=(ARG_SUBDIR, ARG_DAG_ID_OPT,ARG_OUTPUT_PATH),
Review comment:
```suggestion
args=(ARG_SUBDIR, ARG_DAG_ID, ARG_OUTPUT_PATH),
```
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483290746
##########
File path: airflow/cli/cli_parser.py
##########
@@ -778,6 +782,12 @@ class GroupCommand(NamedTuple):
func=lazy_load_command('airflow.cli.commands.dag_command.dag_list_dags'),
args=(ARG_SUBDIR, ARG_OUTPUT),
),
+ ActionCommand(
+ name='generate_kubernetes_pod_yaml',
Review comment:
Actually looks like "next_execution" might need changing as well. We definitely need to decide dash or underscore
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483291220
##########
File path: airflow/cli/cli_parser.py
##########
@@ -142,6 +142,10 @@ def positive_int(value):
("-e", "--end-date"),
help="Override end_date YYYY-MM-DD",
type=parsedate)
+ARG_OUTPUT_PATH = Arg(
+ ("-o", "--output-path",),
+ help="The output for generated yaml files",
+ default="/tmp/airflow_yaml_output/")
Review comment:
@mik-laj has a Valid point
----------------------------------------------------------------
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
[GitHub] [airflow] dimberman commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482230994
##########
File path: tests/cli/commands/test_dag_command.py
##########
@@ -139,6 +139,12 @@ def test_show_dag_print(self):
self.assertIn("graph [label=example_bash_operator labelloc=t rankdir=LR]", out)
self.assertIn("runme_2 -> run_after_loop", out)
+ def test_generate_dag_yaml(self):
+ with contextlib.redirect_stdout(io.StringIO()) as temp_stdout:
+ dag_command.generate_pod_yaml(self.parser.parse_args([
+ 'dags', 'generate_yaml', '--dag-id', 'example_bash_operator']))
+
Review comment:
@mik-laj PTAL added assertions
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483293388
##########
File path: airflow/cli/cli_parser.py
##########
@@ -142,6 +142,10 @@ def positive_int(value):
("-e", "--end-date"),
help="Override end_date YYYY-MM-DD",
type=parsedate)
+ARG_OUTPUT_PATH = Arg(
+ ("-o", "--output-path",),
+ help="The output for generated yaml files",
+ default="/tmp/airflow_yaml_output/")
Review comment:
i.e the subcommand can create a folder example "output_yaml_files" or something in the current directory that stores all the yaml's
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483142394
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -378,6 +380,48 @@ def dag_list_dag_runs(args, dag=None):
print(table)
+@cli_utils.action_logging
+def generate_pod_yaml(args):
+ """Generates yaml files for each task in the DAG. Used for testing output of KubernetesExecutor"""
+
+ from airflow.executors.kubernetes_executor import AirflowKubernetesScheduler, KubeConfig
+ from airflow.kubernetes import pod_generator
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.kubernetes.worker_configuration import WorkerConfiguration
+ from airflow.settings import pod_mutation_hook
+ from airflow.utils import timezone
Review comment:
This imports can go at the top of the file, no ?
----------------------------------------------------------------
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
[GitHub] [airflow] dimberman commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483283773
##########
File path: airflow/cli/cli_parser.py
##########
@@ -778,6 +782,12 @@ class GroupCommand(NamedTuple):
func=lazy_load_command('airflow.cli.commands.dag_command.dag_list_dags'),
args=(ARG_SUBDIR, ARG_OUTPUT),
),
+ ActionCommand(
+ name='generate_kubernetes_pod_yaml',
+ help="Generate YAML files for all tasks in DAG",
+ func=lazy_load_command('airflow.cli.commands.dag_command.generate_pod_yaml'),
+ args=(ARG_SUBDIR, ARG_DAG_ID, ARG_OUTPUT_PATH, ARG_EXEC_DATE),
Review comment:
IMHO execution_date should never be a positional argument. However making this change would be way outside of the scope of this ticket. Can we merge this as is and open a thread on the mailing list for a wider discussion of this?
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483291301
##########
File path: airflow/cli/cli_parser.py
##########
@@ -142,6 +142,10 @@ def positive_int(value):
("-e", "--end-date"),
help="Override end_date YYYY-MM-DD",
type=parsedate)
+ARG_OUTPUT_PATH = Arg(
+ ("-o", "--output-path",),
+ help="The output for generated yaml files",
+ default="/tmp/airflow_yaml_output/")
Review comment:
I agree with Kamil on thjs
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482189488
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -378,6 +379,47 @@ def dag_list_dag_runs(args, dag=None):
print(table)
+@provide_session
+@cli_utils.action_logging
+def generate_pod_yaml(args, session=None):
+ """Generates yaml files for each task in the DAG. Used for testing output of KubernetesExecutor"""
+
+ from airflow.executors.kubernetes_executor import AirflowKubernetesScheduler, KubeConfig
+ from airflow.kubernetes import pod_generator
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.kubernetes.worker_configuration import WorkerConfiguration
+ execution_date = datetime.datetime(2020,11,3)
+ dag = get_dag(subdir=args.subdir, dag_id=args.dag_id)
+ yaml_output_path = args.output_path or "/tmp/airflow_generated_yaml/"
+ kube_config = KubeConfig()
+ for task in dag.tasks:
+ ti = TaskInstance(task, execution_date)
+ pod = PodGenerator.construct_pod(
+ dag_id=args.dag_id,
+ task_id=ti.task_id,
+ pod_id=AirflowKubernetesScheduler._create_pod_id(args.dag_id, ti.task_id),
+ try_number=ti.try_number,
+ date=ti.execution_date,
+ command=ti.command_as_list(),
+ kube_executor_config=PodGenerator.from_obj(ti.executor_config),
+ worker_uuid="worker-config",
+ namespace=kube_config.executor_namespace,
+ worker_config=WorkerConfiguration(kube_config=kube_config).as_pod()
+ )
+ import os
+
+ import yaml
+ from kubernetes.client.api_client import ApiClient
+ api_client = ApiClient()
+ date_string = pod_generator.datetime_to_label_safe_datestring(execution_date)
+ yaml_file_name = f"{args.dag_id}_{ti.task_id}_{date_string}.yml"
+ os.makedirs(os.path.dirname(yaml_output_path), exist_ok=True)
+ with open(yaml_output_path + yaml_file_name, "w") as output:
+ sanitized_pod = api_client.sanitize_for_serialization(pod)
+ output.write(yaml.dump(sanitized_pod))
+ print(f"YAML output can be found at {yaml_output_path}")
Review comment:
```suggestion
print(f"YAML output can be found at {yaml_output_path}", file=sys.err)
```
----------------------------------------------------------------
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
[GitHub] [airflow] dimberman commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482982545
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -378,6 +379,47 @@ def dag_list_dag_runs(args, dag=None):
print(table)
+@cli_utils.action_logging
+def generate_pod_yaml(args):
+ """Generates yaml files for each task in the DAG. Used for testing output of KubernetesExecutor"""
+
+ from airflow.executors.kubernetes_executor import AirflowKubernetesScheduler, KubeConfig
+ from airflow.kubernetes import pod_generator
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.kubernetes.worker_configuration import WorkerConfiguration
+ execution_date = datetime.datetime(2020, 11, 3)
Review comment:
@kaxil since this endpoint is for testing purposes, it didn't make sense to force users to put in an execution date, and it seemed to make sense to hard code it so there would be consistency. Do you feel differently?
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483280382
##########
File path: airflow/cli/cli_parser.py
##########
@@ -142,6 +142,10 @@ def positive_int(value):
("-e", "--end-date"),
help="Override end_date YYYY-MM-DD",
type=parsedate)
+ARG_OUTPUT_PATH = Arg(
+ ("-o", "--output-path",),
+ help="The output for generated yaml files",
+ default="/tmp/airflow_yaml_output/")
Review comment:
I understand the reasons, but I'm afraid it might be more difficult to use because of this. It is common to save files in the current directory by default. If the user wishes to write to tmp, he can execute the following sequence of commands.
```bash
cd /tmp/
mkdir -p airflow_tmp
airflow dags generate_kubernetes_pod_yaml example_bash_operator
ls
```
If we save in / tmp / the user may feel confused. When I unpack or create a file, it expects it to be visible after executing `` ls ''
```bash
cd ~
airflow dags generate_kubernetes_pod_yaml example_bash_operator
ls
# Generated files not visible.
```
In 2020, users have large enough hard drives that we do not have to worry about saving too much data. I think their convenience and productivity are more important.
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483255937
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -378,6 +380,48 @@ def dag_list_dag_runs(args, dag=None):
print(table)
+@cli_utils.action_logging
+def generate_pod_yaml(args):
+ """Generates yaml files for each task in the DAG. Used for testing output of KubernetesExecutor"""
+
+ from kubernetes.client.api_client import ApiClient
Review comment:
We have a similar check for Celery.
<img width="1247" alt="Screenshot 2020-09-03 at 23 10 08" src="https://user-images.githubusercontent.com/12058428/92167409-a14e5280-ee3a-11ea-85f0-6f7059b9a994.png">
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482606210
##########
File path: airflow/cli/cli_parser.py
##########
@@ -142,6 +142,10 @@ def positive_int(value):
("-e", "--end-date"),
help="Override end_date YYYY-MM-DD",
type=parsedate)
+ARG_OUTPUT_PATH = Arg(
+ ("-o", "--output-path",),
+ help="The output for generated yaml files",
Review comment:
This hasn't been fixed @dimberman
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483291762
##########
File path: airflow/cli/cli_parser.py
##########
@@ -142,6 +142,10 @@ def positive_int(value):
("-e", "--end-date"),
help="Override end_date YYYY-MM-DD",
type=parsedate)
+ARG_OUTPUT_PATH = Arg(
+ ("-o", "--output-path",),
+ help="The output for generated yaml files",
+ default="/tmp/airflow_yaml_output/")
Review comment:
@potiuk WDYT
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483253555
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -378,6 +379,47 @@ def dag_list_dag_runs(args, dag=None):
print(table)
+@cli_utils.action_logging
+def generate_pod_yaml(args):
+ """Generates yaml files for each task in the DAG. Used for testing output of KubernetesExecutor"""
+
+ from airflow.executors.kubernetes_executor import AirflowKubernetesScheduler, KubeConfig
+ from airflow.kubernetes import pod_generator
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.kubernetes.worker_configuration import WorkerConfiguration
+ execution_date = datetime.datetime(2020, 11, 3)
Review comment:
Elsewhere execution_date is required. Should we unify it?
```
usage: airflow tasks test [-h] [-n] [--env-vars ENV_VARS] [-m] [-S SUBDIR]
[-t TASK_PARAMS]
dag_id task_id execution_date
```
```
airflow dags test [-h] [--imgcat-dagrun] [--save-dagrun SAVE_DAGRUN]
[--show-dagrun] [-S SUBDIR]
dag_id execution_date
```
```
usage: airflow tasks run [-h] [--cfg-path CFG_PATH] [-f] [-A] [-i] [-I] [-N]
[-l] [-m] [-p PICKLE] [--pool POOL] [--ship-dag]
[-S SUBDIR]
dag_id task_id execution_date
```
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482205398
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -378,6 +379,47 @@ def dag_list_dag_runs(args, dag=None):
print(table)
+@provide_session
+@cli_utils.action_logging
+def generate_pod_yaml(args, session=None):
+ """Generates yaml files for each task in the DAG. Used for testing output of KubernetesExecutor"""
+
+ from airflow.executors.kubernetes_executor import AirflowKubernetesScheduler, KubeConfig
+ from airflow.kubernetes import pod_generator
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.kubernetes.worker_configuration import WorkerConfiguration
+ execution_date = datetime.datetime(2020,11,3)
+ dag = get_dag(subdir=args.subdir, dag_id=args.dag_id)
+ yaml_output_path = args.output_path or "/tmp/airflow_generated_yaml/"
Review comment:
Why one task in a separate file? Wouldn't it be more convenient if there was one file for each DAG? This will prevent you from having to write the data to a file, but you can display it on the screen. Ideally, the YAML displayed on the screen should also have colors. Take a look: `airflow config list`.
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483286901
##########
File path: airflow/cli/cli_parser.py
##########
@@ -778,6 +782,12 @@ class GroupCommand(NamedTuple):
func=lazy_load_command('airflow.cli.commands.dag_command.dag_list_dags'),
args=(ARG_SUBDIR, ARG_OUTPUT),
),
+ ActionCommand(
+ name='generate_kubernetes_pod_yaml',
Review comment:
```suggestion
name='generate-kubernetes-pod-yaml',
```
Commands should use dash as a separator. See: https://github.com/apache/airflow/pull/10669
I forgot one command yet, but I will correct it in a moment.
----------------------------------------------------------------
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
[GitHub] [airflow] dimberman commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482192545
##########
File path: airflow/cli/cli_parser.py
##########
@@ -778,6 +782,12 @@ class GroupCommand(NamedTuple):
func=lazy_load_command('airflow.cli.commands.dag_command.dag_list_dags'),
args=(ARG_SUBDIR, ARG_OUTPUT),
),
+ ActionCommand(
+ name='generate_yaml',
+ help="Generate YAML files for all tasks in DAG",
+ func=lazy_load_command('airflow.cli.commands.dag_command.generate_pod_yaml'),
+ args=(ARG_SUBDIR, ARG_DAG_ID_OPT,ARG_OUTPUT_PATH),
Review comment:
How do I make this not optional?
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483282305
##########
File path: airflow/cli/cli_parser.py
##########
@@ -778,6 +782,12 @@ class GroupCommand(NamedTuple):
func=lazy_load_command('airflow.cli.commands.dag_command.dag_list_dags'),
args=(ARG_SUBDIR, ARG_OUTPUT),
),
+ ActionCommand(
+ name='generate_kubernetes_pod_yaml',
+ help="Generate YAML files for all tasks in DAG",
+ func=lazy_load_command('airflow.cli.commands.dag_command.generate_pod_yaml'),
+ args=(ARG_SUBDIR, ARG_DAG_ID, ARG_OUTPUT_PATH, ARG_EXEC_DATE),
Review comment:
This is not necessary, but should we update the other commands as well? Using CLI is very often based on habits, and I think it's worth making sure that all commands have a similar style.
```
airflow dags test example_bash_operator 2019-01-01
airflow dags generate_kubernetes_pod_yaml example_bash_operator --exec-date 2019-01-01
```
I suspect these two commands may be used by the same people and follow two different conventions. This is very confusing to the user. I am in favor of making this argument optional, but it should be done in all commands, or none of them.
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483283805
##########
File path: airflow/cli/cli_parser.py
##########
@@ -778,6 +782,12 @@ class GroupCommand(NamedTuple):
func=lazy_load_command('airflow.cli.commands.dag_command.dag_list_dags'),
args=(ARG_SUBDIR, ARG_OUTPUT),
),
+ ActionCommand(
+ name='generate_kubernetes_pod_yaml',
+ help="Generate YAML files for all tasks in DAG",
+ func=lazy_load_command('airflow.cli.commands.dag_command.generate_pod_yaml'),
+ args=(ARG_SUBDIR, ARG_DAG_ID, ARG_OUTPUT_PATH, ARG_EXEC_DATE),
Review comment:
Is there any reason why the command `airflow dags generate_kubernetes_pod_yaml` should have exeuction_date optional, and the command `airflow dags test` has this argument required? In my opinion, these two commands have a similar role. These are commands created for development.
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483280382
##########
File path: airflow/cli/cli_parser.py
##########
@@ -142,6 +142,10 @@ def positive_int(value):
("-e", "--end-date"),
help="Override end_date YYYY-MM-DD",
type=parsedate)
+ARG_OUTPUT_PATH = Arg(
+ ("-o", "--output-path",),
+ help="The output for generated yaml files",
+ default="/tmp/airflow_yaml_output/")
Review comment:
I understand the reasons, but I'm afraid it might be more difficult to use because of this. It is common to save files in the current directory by default. If the user wishes to write to tmp, they can execute the following sequence of commands.
```bash
cd /tmp/
mkdir -p airflow_tmp
airflow dags generate_kubernetes_pod_yaml example_bash_operator
ls
```
If we save in /tmp/ the user may feel confused. When I unpack or create a file, I expect results to be visible after executing ``ls``
```bash
cd ~
airflow dags generate_kubernetes_pod_yaml example_bash_operator
ls
# Generated files not visible.
```
In 2020, users have large enough hard drives that we do not have to worry about saving too much data. I think their convenience and productivity are more important.
----------------------------------------------------------------
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
[GitHub] [airflow] dimberman commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483259411
##########
File path: airflow/cli/cli_parser.py
##########
@@ -778,6 +782,12 @@ class GroupCommand(NamedTuple):
func=lazy_load_command('airflow.cli.commands.dag_command.dag_list_dags'),
args=(ARG_SUBDIR, ARG_OUTPUT),
),
+ ActionCommand(
+ name='generate_kubernetes_pod_yaml',
+ help="Generate YAML files for all tasks in DAG",
+ func=lazy_load_command('airflow.cli.commands.dag_command.generate_pod_yaml'),
+ args=(ARG_SUBDIR, ARG_DAG_ID, ARG_OUTPUT_PATH, ARG_EXEC_DATE),
Review comment:
@mik-laj does it need to be required? this is just a diagnostic tool so it shouldn't make a difference if we use a dummy execution_date.
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483286372
##########
File path: airflow/cli/cli_parser.py
##########
@@ -778,6 +782,12 @@ class GroupCommand(NamedTuple):
func=lazy_load_command('airflow.cli.commands.dag_command.dag_list_dags'),
args=(ARG_SUBDIR, ARG_OUTPUT),
),
+ ActionCommand(
+ name='generate_kubernetes_pod_yaml',
+ help="Generate YAML files for all tasks in DAG",
+ func=lazy_load_command('airflow.cli.commands.dag_command.generate_pod_yaml'),
Review comment:
```suggestion
name='dump-pod-yaml',
help="Generate YAML files with K8S Pod definitions.",
description="Generate YAML files with Kubernetes Pod definitions for all tasks in DAG",
```
This command name is too long. It doesn't look good in CLI.
<img width="687" alt="Screenshot 2020-09-04 at 00 25 43" src="https://user-images.githubusercontent.com/12058428/92179595-440bce80-ee45-11ea-8f0b-ed578042e3c3.png">
----------------------------------------------------------------
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
[GitHub] [airflow] dimberman commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483295092
##########
File path: airflow/cli/cli_parser.py
##########
@@ -142,6 +142,10 @@ def positive_int(value):
("-e", "--end-date"),
help="Override end_date YYYY-MM-DD",
type=parsedate)
+ARG_OUTPUT_PATH = Arg(
+ ("-o", "--output-path",),
+ help="The output for generated yaml files",
+ default="/tmp/airflow_yaml_output/")
Review comment:
Done :)
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482193145
##########
File path: airflow/cli/cli_parser.py
##########
@@ -778,6 +783,12 @@ class GroupCommand(NamedTuple):
func=lazy_load_command('airflow.cli.commands.dag_command.dag_list_dags'),
args=(ARG_SUBDIR, ARG_OUTPUT),
),
+ ActionCommand(
+ name='generate_yaml',
Review comment:
What do you think to add this to the new ``kubernetes`` group? We currently have sections for celery executor.
```
Celery components
airflow celery flower - Start a Celery Flower
airflow celery stop - Stop the Celery worker gracefully
airflow celery worker - Start a Celery worker node
```
----------------------------------------------------------------
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
[GitHub] [airflow] dimberman commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483291227
##########
File path: airflow/cli/cli_parser.py
##########
@@ -142,6 +142,10 @@ def positive_int(value):
("-e", "--end-date"),
help="Override end_date YYYY-MM-DD",
type=parsedate)
+ARG_OUTPUT_PATH = Arg(
+ ("-o", "--output-path",),
+ help="The output for generated yaml files",
+ default="/tmp/airflow_yaml_output/")
Review comment:
I would personally be really annoyed if I ran this command and then suddenly had to clean up 20 yaml files in my current directory
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483255401
##########
File path: airflow/cli/cli_parser.py
##########
@@ -142,6 +142,10 @@ def positive_int(value):
("-e", "--end-date"),
help="Override end_date YYYY-MM-DD",
type=parsedate)
+ARG_OUTPUT_PATH = Arg(
+ ("-o", "--output-path",),
+ help="The output for generated yaml files",
+ default="/tmp/airflow_yaml_output/")
Review comment:
Wouldn't it be better to write in Current Working Directory? This will provide easier access to these files. Most linux utilities use CWD by default.
```
tar xf pathtoarchive/archive.tar
```
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483142837
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -378,6 +380,48 @@ def dag_list_dag_runs(args, dag=None):
print(table)
+@cli_utils.action_logging
+def generate_pod_yaml(args):
+ """Generates yaml files for each task in the DAG. Used for testing output of KubernetesExecutor"""
+
+ from airflow.executors.kubernetes_executor import AirflowKubernetesScheduler, KubeConfig
+ from airflow.kubernetes import pod_generator
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.kubernetes.worker_configuration import WorkerConfiguration
+ from airflow.settings import pod_mutation_hook
+ from airflow.utils import timezone
+ execution_date = args.exec_date or timezone.utcnow()
+ dag = get_dag(subdir=args.subdir, dag_id=args.dag_id)
+ yaml_output_path = args.output_path or "/tmp/airflow_generated_yaml/"
+ kube_config = KubeConfig()
+ for task in dag.tasks:
+ ti = TaskInstance(task, execution_date)
+ pod = PodGenerator.construct_pod(
+ dag_id=args.dag_id,
+ task_id=ti.task_id,
+ pod_id=AirflowKubernetesScheduler._create_pod_id( # pylint: disable=W0212
+ args.dag_id, ti.task_id),
+ try_number=ti.try_number,
+ date=ti.execution_date,
+ command=ti.command_as_list(),
+ kube_executor_config=PodGenerator.from_obj(ti.executor_config),
+ worker_uuid="worker-config",
+ namespace=kube_config.executor_namespace,
+ worker_config=WorkerConfiguration(kube_config=kube_config).as_pod()
+ )
+ pod_mutation_hook(pod)
+
+ from kubernetes.client.api_client import ApiClient
Review comment:
same here, import in the middle of the file / function ? Any particular reason?
----------------------------------------------------------------
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
[GitHub] [airflow] dimberman commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482236372
##########
File path: airflow/cli/cli_parser.py
##########
@@ -778,6 +783,12 @@ class GroupCommand(NamedTuple):
func=lazy_load_command('airflow.cli.commands.dag_command.dag_list_dags'),
args=(ARG_SUBDIR, ARG_OUTPUT),
),
+ ActionCommand(
+ name='generate_yaml',
Review comment:
@mik-laj so create a section for one command? Seems like a bit of overkill...
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482191020
##########
File path: tests/cli/commands/test_dag_command.py
##########
@@ -139,6 +139,12 @@ def test_show_dag_print(self):
self.assertIn("graph [label=example_bash_operator labelloc=t rankdir=LR]", out)
self.assertIn("runme_2 -> run_after_loop", out)
+ def test_generate_dag_yaml(self):
+ with contextlib.redirect_stdout(io.StringIO()) as temp_stdout:
+ dag_command.generate_pod_yaml(self.parser.parse_args([
+ 'dags', 'generate_yaml', '--dag-id', 'example_bash_operator']))
+
Review comment:
Can you add even one assertion?
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483291301
##########
File path: airflow/cli/cli_parser.py
##########
@@ -142,6 +142,10 @@ def positive_int(value):
("-e", "--end-date"),
help="Override end_date YYYY-MM-DD",
type=parsedate)
+ARG_OUTPUT_PATH = Arg(
+ ("-o", "--output-path",),
+ help="The output for generated yaml files",
+ default="/tmp/airflow_yaml_output/")
Review comment:
I agree with Kamil on this.
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483290456
##########
File path: airflow/cli/cli_parser.py
##########
@@ -778,6 +782,12 @@ class GroupCommand(NamedTuple):
func=lazy_load_command('airflow.cli.commands.dag_command.dag_list_dags'),
args=(ARG_SUBDIR, ARG_OUTPUT),
),
+ ActionCommand(
+ name='generate_kubernetes_pod_yaml',
Review comment:
Good point
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483292253
##########
File path: airflow/cli/cli_parser.py
##########
@@ -142,6 +142,10 @@ def positive_int(value):
("-e", "--end-date"),
help="Override end_date YYYY-MM-DD",
type=parsedate)
+ARG_OUTPUT_PATH = Arg(
+ ("-o", "--output-path",),
+ help="The output for generated yaml files",
+ default="/tmp/airflow_yaml_output/")
Review comment:
You could output to a directory in the current directory. That way you just need to delete the folder if that is the concern
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj edited a comment on pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj edited a comment on pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#issuecomment-686808891
Last thing, can you add more docs? I am thinking of a sentence or two that shows an example use case. Without it, this command will only be available in the reference documentation, and it is not very user-friendly.
https://airflow.readthedocs.io/en/latest/executor/kubernetes.html
we have a similar section on plugins page:
> To troubleshoot issue with plugins, you can use airflow plugins command. This command dumps information about loaded plugins.
https://airflow.readthedocs.io/en/latest/plugins.html
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483258771
##########
File path: airflow/cli/cli_parser.py
##########
@@ -778,6 +782,12 @@ class GroupCommand(NamedTuple):
func=lazy_load_command('airflow.cli.commands.dag_command.dag_list_dags'),
args=(ARG_SUBDIR, ARG_OUTPUT),
),
+ ActionCommand(
+ name='generate_kubernetes_pod_yaml',
+ help="Generate YAML files for all tasks in DAG",
+ func=lazy_load_command('airflow.cli.commands.dag_command.generate_pod_yaml'),
+ args=(ARG_SUBDIR, ARG_DAG_ID, ARG_OUTPUT_PATH, ARG_EXEC_DATE),
Review comment:
```suggestion
args=(ARG_SUBDIR, ARG_DAG_ID, ARG_OUTPUT_PATH, ARG_EXECUTION_DATE),
```
This will surely make this argument required.
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482983852
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -378,6 +379,47 @@ def dag_list_dag_runs(args, dag=None):
print(table)
+@cli_utils.action_logging
+def generate_pod_yaml(args):
+ """Generates yaml files for each task in the DAG. Used for testing output of KubernetesExecutor"""
+
+ from airflow.executors.kubernetes_executor import AirflowKubernetesScheduler, KubeConfig
+ from airflow.kubernetes import pod_generator
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.kubernetes.worker_configuration import WorkerConfiguration
+ execution_date = datetime.datetime(2020, 11, 3)
Review comment:
I would still think we can use `timezone.utcnow()` as execution date instead of `datetime.datetime(2020, 11, 3)`
where timezone.utcnow is https://github.com/apache/airflow/blob/master/airflow/utils/timezone.py#L52
----------------------------------------------------------------
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
[GitHub] [airflow] dimberman commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482191227
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -378,6 +379,47 @@ def dag_list_dag_runs(args, dag=None):
print(table)
+@provide_session
+@cli_utils.action_logging
+def generate_pod_yaml(args, session=None):
+ """Generates yaml files for each task in the DAG. Used for testing output of KubernetesExecutor"""
+
+ from airflow.executors.kubernetes_executor import AirflowKubernetesScheduler, KubeConfig
+ from airflow.kubernetes import pod_generator
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.kubernetes.worker_configuration import WorkerConfiguration
+ execution_date = datetime.datetime(2020,11,3)
+ dag = get_dag(subdir=args.subdir, dag_id=args.dag_id)
+ yaml_output_path = args.output_path or "/tmp/airflow_generated_yaml/"
Review comment:
@mik-laj would people want that as stdout? The idea is that it would create a file per-task.
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483291220
##########
File path: airflow/cli/cli_parser.py
##########
@@ -142,6 +142,10 @@ def positive_int(value):
("-e", "--end-date"),
help="Override end_date YYYY-MM-DD",
type=parsedate)
+ARG_OUTPUT_PATH = Arg(
+ ("-o", "--output-path",),
+ help="The output for generated yaml files",
+ default="/tmp/airflow_yaml_output/")
Review comment:
Valid point
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482606828
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -378,6 +379,47 @@ def dag_list_dag_runs(args, dag=None):
print(table)
+@cli_utils.action_logging
+def generate_pod_yaml(args):
+ """Generates yaml files for each task in the DAG. Used for testing output of KubernetesExecutor"""
+
+ from airflow.executors.kubernetes_executor import AirflowKubernetesScheduler, KubeConfig
+ from airflow.kubernetes import pod_generator
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.kubernetes.worker_configuration import WorkerConfiguration
+ execution_date = datetime.datetime(2020, 11, 3)
+ dag = get_dag(subdir=args.subdir, dag_id=args.dag_id)
+ yaml_output_path = args.output_path or "/tmp/airflow_generated_yaml/"
+ kube_config = KubeConfig()
+ for task in dag.tasks:
+ ti = TaskInstance(task, execution_date)
+ pod = PodGenerator.construct_pod(
+ dag_id=args.dag_id,
+ task_id=ti.task_id,
+ pod_id=AirflowKubernetesScheduler._create_pod_id( # pylint: disable=W0212
+ args.dag_id, ti.task_id),
+ try_number=ti.try_number,
+ date=ti.execution_date,
+ command=ti.command_as_list(),
+ kube_executor_config=PodGenerator.from_obj(ti.executor_config),
+ worker_uuid="worker-config",
+ namespace=kube_config.executor_namespace,
+ worker_config=WorkerConfiguration(kube_config=kube_config).as_pod()
+ )
+ import os
+
+ import yaml
+ from kubernetes.client.api_client import ApiClient
Review comment:
Why are these here ??? let's properly format them (os and yaml can go at the top of the file)
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483247921
##########
File path: tests/cli/commands/test_dag_command.py
##########
@@ -139,6 +140,19 @@ def test_show_dag_print(self):
self.assertIn("graph [label=example_bash_operator labelloc=t rankdir=LR]", out)
self.assertIn("runme_2 -> run_after_loop", out)
+ def test_generate_dag_yaml(self):
+
+ directory = "/tmp/airflow_dry_run_test/"
Review comment:
Can you use [TemporaryDirectory](https://docs.python.org/3/library/tempfile.html#tempfile.TemporaryDirectory) to avoid side-effect?
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483248984
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -378,6 +380,48 @@ def dag_list_dag_runs(args, dag=None):
print(table)
+@cli_utils.action_logging
+def generate_pod_yaml(args):
+ """Generates yaml files for each task in the DAG. Used for testing output of KubernetesExecutor"""
+
+ from kubernetes.client.api_client import ApiClient
Review comment:
This is a command that is intended for the end user. Do you think it's worth adding a friendly message that will be displayed when Kubernetes is not installed or not configured?
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482189650
##########
File path: airflow/cli/cli_parser.py
##########
@@ -778,6 +782,12 @@ class GroupCommand(NamedTuple):
func=lazy_load_command('airflow.cli.commands.dag_command.dag_list_dags'),
args=(ARG_SUBDIR, ARG_OUTPUT),
),
+ ActionCommand(
+ name='generate_yaml',
+ help="Generate YAML files for all tasks in DAG",
+ func=lazy_load_command('airflow.cli.commands.dag_command.generate_pod_yaml'),
+ args=(ARG_SUBDIR, ARG_DAG_ID_OPT,ARG_OUTPUT_PATH),
Review comment:
might be an error, should not be optional
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482190501
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -378,6 +379,47 @@ def dag_list_dag_runs(args, dag=None):
print(table)
+@provide_session
+@cli_utils.action_logging
+def generate_pod_yaml(args, session=None):
+ """Generates yaml files for each task in the DAG. Used for testing output of KubernetesExecutor"""
+
+ from airflow.executors.kubernetes_executor import AirflowKubernetesScheduler, KubeConfig
+ from airflow.kubernetes import pod_generator
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.kubernetes.worker_configuration import WorkerConfiguration
+ execution_date = datetime.datetime(2020,11,3)
+ dag = get_dag(subdir=args.subdir, dag_id=args.dag_id)
+ yaml_output_path = args.output_path or "/tmp/airflow_generated_yaml/"
Review comment:
Should be standard output by default?
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483251211
##########
File path: airflow/cli/cli_parser.py
##########
@@ -778,6 +783,12 @@ class GroupCommand(NamedTuple):
func=lazy_load_command('airflow.cli.commands.dag_command.dag_list_dags'),
args=(ARG_SUBDIR, ARG_OUTPUT),
),
+ ActionCommand(
+ name='generate_yaml',
Review comment:
Ok. In the future, we can move this to a separate group.
----------------------------------------------------------------
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
[GitHub] [airflow] dimberman merged pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
dimberman merged pull request #10677:
URL: https://github.com/apache/airflow/pull/10677
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#issuecomment-686808891
Last thing, can you add more docs? I am thinking of a sentence or two that shows an example use case. Without it, this command will only be available in the reference documentation, and it is not very user-friendly.
https://airflow.readthedocs.io/en/latest/executor/kubernetes.html
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482189219
##########
File path: airflow/cli/cli_parser.py
##########
@@ -778,6 +782,12 @@ class GroupCommand(NamedTuple):
func=lazy_load_command('airflow.cli.commands.dag_command.dag_list_dags'),
args=(ARG_SUBDIR, ARG_OUTPUT),
),
+ ActionCommand(
+ name='generate_yaml',
+ help="Generate YAML files for all tasks in DAG",
+ func=lazy_load_command('airflow.cli.commands.dag_command.generate_pod_yaml'),
+ args=(ARG_SUBDIR, ARG_DAG_ID_OPT,ARG_OUTPUT_PATH),
Review comment:
Is the DAG ID really optional?
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482188729
##########
File path: airflow/cli/cli_parser.py
##########
@@ -142,6 +142,10 @@ def positive_int(value):
("-e", "--end-date"),
help="Override end_date YYYY-MM-DD",
type=parsedate)
+ARG_OUTPUT_PATH = Arg(
+ ("-o", "--output-path",),
+ help="The output for generated yaml files",
Review comment:
```suggestion
help="The output for generated yaml files",
type=argparse.FileType('w')
```
See: https://docs.python.org/3/library/argparse.html#filetype-objects
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483292539
##########
File path: airflow/cli/cli_parser.py
##########
@@ -778,6 +782,12 @@ class GroupCommand(NamedTuple):
func=lazy_load_command('airflow.cli.commands.dag_command.dag_list_dags'),
args=(ARG_SUBDIR, ARG_OUTPUT),
),
+ ActionCommand(
+ name='generate_kubernetes_pod_yaml',
Review comment:
PR: https://github.com/apache/airflow/pull/10720
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482606477
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -378,6 +379,47 @@ def dag_list_dag_runs(args, dag=None):
print(table)
+@cli_utils.action_logging
+def generate_pod_yaml(args):
+ """Generates yaml files for each task in the DAG. Used for testing output of KubernetesExecutor"""
+
+ from airflow.executors.kubernetes_executor import AirflowKubernetesScheduler, KubeConfig
+ from airflow.kubernetes import pod_generator
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.kubernetes.worker_configuration import WorkerConfiguration
+ execution_date = datetime.datetime(2020, 11, 3)
Review comment:
Why is the execution_date hardcoded ?
----------------------------------------------------------------
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
[GitHub] [airflow] dimberman commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r483259925
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -378,6 +380,48 @@ def dag_list_dag_runs(args, dag=None):
print(table)
+@cli_utils.action_logging
+def generate_pod_yaml(args):
+ """Generates yaml files for each task in the DAG. Used for testing output of KubernetesExecutor"""
+
+ from kubernetes.client.api_client import ApiClient
Review comment:
@mik-laj good idea will implement.
----------------------------------------------------------------
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
[GitHub] [airflow] dimberman commented on a change in pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
Posted by GitBox <gi...@apache.org>.
dimberman commented on a change in pull request #10677:
URL: https://github.com/apache/airflow/pull/10677#discussion_r482230802
##########
File path: airflow/cli/commands/dag_command.py
##########
@@ -378,6 +379,47 @@ def dag_list_dag_runs(args, dag=None):
print(table)
+@provide_session
+@cli_utils.action_logging
+def generate_pod_yaml(args, session=None):
+ """Generates yaml files for each task in the DAG. Used for testing output of KubernetesExecutor"""
+
+ from airflow.executors.kubernetes_executor import AirflowKubernetesScheduler, KubeConfig
+ from airflow.kubernetes import pod_generator
+ from airflow.kubernetes.pod_generator import PodGenerator
+ from airflow.kubernetes.worker_configuration import WorkerConfiguration
+ execution_date = datetime.datetime(2020,11,3)
+ dag = get_dag(subdir=args.subdir, dag_id=args.dag_id)
+ yaml_output_path = args.output_path or "/tmp/airflow_generated_yaml/"
Review comment:
@mik-laj because usually if I'm debugging I'm debugging a specific task. We can add a "join_in_one_file" option if there is interest, but that can be a separate commit.
----------------------------------------------------------------
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