[GitHub] [airflow] potiuk commented on pull request #10718: Switches to better BATS asserts
potiuk commented on pull request #10718: URL: https://github.com/apache/airflow/pull/10718#issuecomment-686939689 All green, just quarantine tests failed :) 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: us...@infra.apache.org
[GitHub] [airflow] yuqian90 commented on pull request #10153: [AIP-34] TaskGroup: A UI task grouping concept as an alternative to SubDagOperator
yuqian90 commented on pull request #10153: URL: https://github.com/apache/airflow/pull/10153#issuecomment-686895896 Updated the title and summary to make it clearer the PR is ready for review. 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: us...@infra.apache.org
[GitHub] [airflow] aranjanthakur commented on a change in pull request #10261: 10166 : Add capability to specify gunicorn access log format for airflow
aranjanthakur commented on a change in pull request #10261: URL: https://github.com/apache/airflow/pull/10261#discussion_r483374690 ## File path: airflow/config_templates/config.yml ## @@ -861,6 +861,13 @@ type: string example: ~ default: "-" +- name: access_logformat + description: | +Access log format for gunicorn webserver + version_added: ~ + type: string + example: ~ + default: "%%(h)s %%(l)s %%(u)s %%(t)s \"%%(r)s\" %%(s)s %%(b)s \"%%(f)s\" \"%%(a)s\"" Review comment: Made changes, so that we don't need to specify default config. If empty value is passed, default value for gunicorn will be used. 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: us...@infra.apache.org
[GitHub] [airflow] aranjanthakur commented on a change in pull request #10261: 10166 : Add capability to specify gunicorn access log format for airflow
aranjanthakur commented on a change in pull request #10261: URL: https://github.com/apache/airflow/pull/10261#discussion_r483374775 ## File path: airflow/config_templates/config.yml ## @@ -861,6 +861,13 @@ type: string example: ~ default: "-" +- name: access_logformat + description: | +Access log format for gunicorn webserver Review comment: Added link to documentation 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: us...@infra.apache.org
[GitHub] [airflow] Shivarp1 opened a new issue #10722: KubernetesPodOperator with XCom enabled errors: 403 forbidden
Shivarp1 opened a new issue #10722:
URL: https://github.com/apache/airflow/issues/10722
Apache Airflow version: 1.10.12
kubernetes Version: 10.0.1
OpenAPI spec version: v1.14.5
OS: Linux Red Hat 7.x
---
Enabling the do_xcom=True with default configuration parameters for pod
throws the 403 forbidden error as below
--
HTTP response body:
{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"pods
\"xyz..\" is forbidden: failed quota: default: must specify
requests.memory","reason":"Forbidden","details":{"name":"xyz..","kind":"pods"},"code":403}
---
*What you expected to happen**: Create the default side car object with
alpine image and launch the pod
--
What do you think went wrong?
In airflow.kubernetes.pod_generator the PodDefaults.SIDECAR_CONTAINER's
resources does not include requests memory and limits memory.cpu. These are
checked by the Kubernetes client api later and throws this error
--
**How to reproduce it**: A simple XCom test reproduces it
--
**How to resolve it**
Add requests memory, and limits to the resources to the
PodDefaults.SIDECAR_CONTAINER as below..
--
resources=k8s.V1ResourceRequirements(
requests={
"cpu": "10m",
"memory":"32Mi"
},
limits={
"cpu": "0.25",
"memory":"32Mi"
}
),
--
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:
us...@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on issue #10722: KubernetesPodOperator with XCom enabled errors: 403 forbidden
boring-cyborg[bot] commented on issue #10722: URL: https://github.com/apache/airflow/issues/10722#issuecomment-686869781 Thanks for opening your first issue here! Be sure to follow the issue template! 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: us...@infra.apache.org
[airflow] branch v1-10-test updated: Add generate_yaml command to easily test KubernetesExecutor before deploying pods (#10677)
This is an automated email from the ASF dual-hosted git repository.
dimberman pushed a commit to branch v1-10-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/v1-10-test by this push:
new c01c6b7 Add generate_yaml command to easily test KubernetesExecutor
before deploying pods (#10677)
c01c6b7 is described below
commit c01c6b79bf4fbe512ed3e969a68a28ab75c66355
Author: Daniel Imberman
AuthorDate: Thu Sep 3 19:37:11 2020 -0700
Add generate_yaml command to easily test KubernetesExecutor before
deploying pods (#10677)
---
airflow/bin/cli.py | 51 +++
1 file changed, 51 insertions(+)
diff --git a/airflow/bin/cli.py b/airflow/bin/cli.py
index c621a3a..302ab55 100644
--- a/airflow/bin/cli.py
+++ b/airflow/bin/cli.py
@@ -31,6 +31,7 @@ import subprocess
import textwrap
import random
import string
+import yaml
from collections import OrderedDict
from importlib import import_module
@@ -1249,6 +1250,41 @@ def _serve_logs(env, skip_serve_logs=False):
@cli_utils.action_logging
+def kubernetes_generate_dag_yaml(args):
+from airflow.executors.kubernetes_executor import
AirflowKubernetesScheduler, KubeConfig
+from airflow.kubernetes.pod_generator import PodGenerator
+from airflow.kubernetes.worker_configuration import WorkerConfiguration
+from kubernetes.client.api_client import ApiClient
+dag = get_dag(args)
+yaml_output_path = args.output_path
+kube_config = KubeConfig()
+for task in dag.tasks:
+ti = TaskInstance(task, args.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()
+)
+api_client = ApiClient()
+date_string =
AirflowKubernetesScheduler._datetime_to_label_safe_datestring( # pylint:
disable=W0212
+args.execution_date)
+yaml_file_name = "{}_{}_{}.yml".format(args.dag_id, ti.task_id,
date_string)
+os.makedirs(os.path.dirname(yaml_output_path +
"/airflow_yaml_output/"), exist_ok=True)
+with open(yaml_output_path + "/airflow_yaml_output/" + yaml_file_name,
"w") as output:
+sanitized_pod = api_client.sanitize_for_serialization(pod)
+output.write(yaml.dump(sanitized_pod))
+print("YAML output can be found at
{}/airflow_yaml_output/".format(yaml_output_path))
+
+
+@cli_utils.action_logging
def worker(args):
env = os.environ.copy()
env['AIRFLOW_HOME'] = settings.AIRFLOW_HOME
@@ -2172,6 +2208,11 @@ class CLIFactory(object):
'execution_date': Arg(
("execution_date",), help="The execution date of the DAG",
type=parsedate),
+'output_path': Arg(
+('-o', '--output-path'),
+help="output path for yaml file",
+default=os.getcwd()
+),
'task_regex': Arg(
("-t", "--task_regex"),
"The regex to filter specific task_ids to backfill (optional)"),
@@ -2695,6 +2736,16 @@ class CLIFactory(object):
'help': "List the tasks within a DAG",
'args': ('dag_id', 'tree', 'subdir'),
}, {
+'func': kubernetes_generate_dag_yaml,
+'help': "List dag runs given a DAG id. If state option is given,
it will only"
+"search for all the dagruns with the given state. "
+"If no_backfill option is given, it will filter out"
+"all backfill dagruns for given dag id.",
+'args': (
+'dag_id', 'output_path', 'subdir', 'execution_date'
+)
+
+}, {
'func': clear,
'help': "Clear a set of task instance, as if they never ran",
'args': (
[airflow] branch v1-10-test updated: Add generate_yaml command to easily test KubernetesExecutor before deploying pods (#10677)
This is an automated email from the ASF dual-hosted git repository.
dimberman pushed a commit to branch v1-10-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/v1-10-test by this push:
new c01c6b7 Add generate_yaml command to easily test KubernetesExecutor
before deploying pods (#10677)
c01c6b7 is described below
commit c01c6b79bf4fbe512ed3e969a68a28ab75c66355
Author: Daniel Imberman
AuthorDate: Thu Sep 3 19:37:11 2020 -0700
Add generate_yaml command to easily test KubernetesExecutor before
deploying pods (#10677)
---
airflow/bin/cli.py | 51 +++
1 file changed, 51 insertions(+)
diff --git a/airflow/bin/cli.py b/airflow/bin/cli.py
index c621a3a..302ab55 100644
--- a/airflow/bin/cli.py
+++ b/airflow/bin/cli.py
@@ -31,6 +31,7 @@ import subprocess
import textwrap
import random
import string
+import yaml
from collections import OrderedDict
from importlib import import_module
@@ -1249,6 +1250,41 @@ def _serve_logs(env, skip_serve_logs=False):
@cli_utils.action_logging
+def kubernetes_generate_dag_yaml(args):
+from airflow.executors.kubernetes_executor import
AirflowKubernetesScheduler, KubeConfig
+from airflow.kubernetes.pod_generator import PodGenerator
+from airflow.kubernetes.worker_configuration import WorkerConfiguration
+from kubernetes.client.api_client import ApiClient
+dag = get_dag(args)
+yaml_output_path = args.output_path
+kube_config = KubeConfig()
+for task in dag.tasks:
+ti = TaskInstance(task, args.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()
+)
+api_client = ApiClient()
+date_string =
AirflowKubernetesScheduler._datetime_to_label_safe_datestring( # pylint:
disable=W0212
+args.execution_date)
+yaml_file_name = "{}_{}_{}.yml".format(args.dag_id, ti.task_id,
date_string)
+os.makedirs(os.path.dirname(yaml_output_path +
"/airflow_yaml_output/"), exist_ok=True)
+with open(yaml_output_path + "/airflow_yaml_output/" + yaml_file_name,
"w") as output:
+sanitized_pod = api_client.sanitize_for_serialization(pod)
+output.write(yaml.dump(sanitized_pod))
+print("YAML output can be found at
{}/airflow_yaml_output/".format(yaml_output_path))
+
+
+@cli_utils.action_logging
def worker(args):
env = os.environ.copy()
env['AIRFLOW_HOME'] = settings.AIRFLOW_HOME
@@ -2172,6 +2208,11 @@ class CLIFactory(object):
'execution_date': Arg(
("execution_date",), help="The execution date of the DAG",
type=parsedate),
+'output_path': Arg(
+('-o', '--output-path'),
+help="output path for yaml file",
+default=os.getcwd()
+),
'task_regex': Arg(
("-t", "--task_regex"),
"The regex to filter specific task_ids to backfill (optional)"),
@@ -2695,6 +2736,16 @@ class CLIFactory(object):
'help': "List the tasks within a DAG",
'args': ('dag_id', 'tree', 'subdir'),
}, {
+'func': kubernetes_generate_dag_yaml,
+'help': "List dag runs given a DAG id. If state option is given,
it will only"
+"search for all the dagruns with the given state. "
+"If no_backfill option is given, it will filter out"
+"all backfill dagruns for given dag id.",
+'args': (
+'dag_id', 'output_path', 'subdir', 'execution_date'
+)
+
+}, {
'func': clear,
'help': "Clear a set of task instance, as if they never ran",
'args': (
[GitHub] [airflow] dimberman opened a new pull request #10393: Simplify the K8sExecutor and K8sPodOperator
dimberman opened a new pull request #10393: URL: https://github.com/apache/airflow/pull/10393 As discussed in an earlier email thread, this PR removes much of the configuration elements of the K8sExecutor for a much simpler design. --- **^ 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: us...@infra.apache.org
[GitHub] [airflow] jaketf edited a comment on issue #10687: Make region parameter required in Google Dataproc operators and hooks
jaketf edited a comment on issue #10687: URL: https://github.com/apache/airflow/issues/10687#issuecomment-686848519 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: us...@infra.apache.org
[GitHub] [airflow] jaketf edited a comment on issue #10687: Make region parameter required in Google Dataproc operators and hooks
jaketf edited a comment on issue #10687: URL: https://github.com/apache/airflow/issues/10687#issuecomment-686848519 @turbaszek I do not think we should unify naming on location as this would not line up with the REST API or the python client lib. the right thing to do here is a bit confusing looking at the [REST API docs](https://cloud.google.com/dataproc/docs/reference/rest) Some dataproc resources (e.g. job / cluster) only have `regions/` endpoints. Other dataproc resources (workflow templates) have BOTH `regions/` and `locations/` endpoints. In these cases the python client lib uses full `parent` parameter ([example](https://googleapis.dev/python/dataproc/latest/dataproc_v1beta2/services.html#google.cloud.dataproc_v1beta2.services.workflow_template_service.WorkflowTemplateServiceClient.instantiate_inline_workflow_template)). This is clearly intentional but I'm not entirely sure the distinction of when one would use `locations/` over the `regions/` endpoint. I imagine a user who understands this difference would want the flexibility to use either endpoint from their airflow DAGs and thus for hook methods / operators that interact with those resources we should provide mutually exclusive `region` AND `location` parameters OR take the same approach as python client lib and use`parent`. It's my hunch that this locations endpoint is a future proofing thing. Any given dataproc job runs only on a single dataproc cluster which will in only live in a single zone but we submit jobs to a regional endpoint and can OPTIONALLY specify a zone in the job / cluster config in the request or use [dataproc auto-zone placement](https://cloud.google.com/dataproc/docs/concepts/configuring-clusters/auto-zone) hence why regional endpoints for ultimately zonal resources. This means "unifying naming on location" will be less explicitly mapping to the API endpoint used for those resources that actually use the region endpoint and this might lead to confusing code like `submit_job(region=self.location,...)` 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: us...@infra.apache.org
[GitHub] [airflow] jaketf edited a comment on issue #10687: Make region parameter required in Google Dataproc operators and hooks
jaketf edited a comment on issue #10687: URL: https://github.com/apache/airflow/issues/10687#issuecomment-686848519 @turbaszek I do not think we should unify naming on location as this would not line up with the REST API or the python client lib. the right thing to do here is a bit confusing looking at the [REST API docs](https://cloud.google.com/dataproc/docs/reference/rest) Some dataproc resources (e.g. job / cluster) only have `regions/` endpoints. Other dataproc resources (workflow templates) have BOTH `regions/` and `locations/` endpoints. In these cases the python client lib uses full `parent` parameter. This is clearly intentional but I'm not entirely sure the distinction of when one would use `locations/` over the `regions/` endpoint. I imagine a user who understands this difference would want the flexibility to use either endpoint from their airflow DAGs and thus for hook methods / operators that interact with those resources we should provide mutually exclusive `region` AND `location` parameters OR take the same approach as python client lib and use`parent`. It's my hunch that this locations endpoint is a future proofing thing. Any given dataproc job runs only on a single dataproc cluster which will in only live in a single zone but we submit jobs to a regional endpoint and can OPTIONALLY specify a zone in the job / cluster config in the request or use [dataproc auto-zone placement](https://cloud.google.com/dataproc/docs/concepts/configuring-clusters/auto-zone) hence why regional endpoints for ultimately zonal resources. This means "unifying naming on location" will be less explicitly mapping to the API endpoint used for those resources that actually use the region endpoint and this might lead to confusing code like `submit_job(region=self.location,...)` 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: us...@infra.apache.org
[GitHub] [airflow] jaketf edited a comment on issue #10687: Make region parameter required in Google Dataproc operators and hooks
jaketf edited a comment on issue #10687: URL: https://github.com/apache/airflow/issues/10687#issuecomment-686848519 @turbaszek I do not think we should unify naming on location as this would not line up with the REST API or the python client lib. the right thing to do here is a bit confusing looking at the [REST API docs](https://cloud.google.com/dataproc/docs/reference/rest) Some dataproc resources (e.g. job / cluster) only have `regions/` endpoints. Other dataproc resources (workflow templates) have BOTH `regions/` and `locations/` endpoints. In these cases the python client lib uses full `parent` parameter. This is clearly intentional but I'm not entirely sure the distinction of when one would use `locations/` over the `regions/` endpoint. I imagine a user who understands this difference would want the flexibility to use either endpoint from their airflow DAGs and thus for hook methods / operators that interact with those resources we should provide mutually exclusive region AND location parameters. It's my hunch that this locations endpoint is a future proofing thing. Any given dataproc job runs only on a single dataproc cluster which will in only live in a single zone but we submit jobs to a regional endpoint and can OPTIONALLY specify a zone in the job / cluster config in the request or use [dataproc auto-zone placement](https://cloud.google.com/dataproc/docs/concepts/configuring-clusters/auto-zone) hence why regional endpoints for ultimately zonal resources. This means "unifying naming on location" will be less explicitly mapping to the API endpoint used for those resources that actually use the region endpoint and this might lead to confusing code like `submit_job(region=self.location,...)` 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: us...@infra.apache.org
[GitHub] [airflow] Adil-Ibragimov commented on pull request #10315: Add retry_only_on_pod_launching_failure and log_container_statuses_on…
Adil-Ibragimov commented on pull request #10315: URL: https://github.com/apache/airflow/pull/10315#issuecomment-686852023 @ashb ping! 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: us...@infra.apache.org
[GitHub] [airflow] dimberman closed pull request #10393: Simplify the K8sExecutor and K8sPodOperator
dimberman closed pull request #10393: URL: https://github.com/apache/airflow/pull/10393 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: us...@infra.apache.org
[GitHub] [airflow] jaketf commented on issue #10687: Make region parameter required in Google Dataproc operators and hooks
jaketf commented on issue #10687: URL: https://github.com/apache/airflow/issues/10687#issuecomment-686850162 I will reach out to dataproc eng for their perspective. 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: us...@infra.apache.org
[GitHub] [airflow] jaketf edited a comment on issue #10687: Make region parameter required in Google Dataproc operators and hooks
jaketf edited a comment on issue #10687: URL: https://github.com/apache/airflow/issues/10687#issuecomment-686848519 @turbaszek I do not think we should unify naming on location as this would not line up with the REST API or the python client lib. the right thing to do here is a bit confusing looking at the [REST API docs](https://cloud.google.com/dataproc/docs/reference/rest) Some dataproc resources (e.g. job / cluster) only have `regions/` endpoints. Other dataproc resources (workflow templates) have BOTH `regions/` and `locations/` endpoints. This is clearly intentional but I'm not entirely sure the distinction of when one would use `locations/` over the `regions/` endpoint. I imagine a user who understands this difference would want the flexibility to use either endpoint from their airflow DAGs and thus for hook methods / operators that interact with those resources we should provide mutually exclusive region AND location parameters. It's my hunch that this locations endpoint is a future proofing thing. Any given dataproc job runs only on a single dataproc cluster which will in only live in a single zone but we submit jobs to a regional endpoint and can OPTIONALLY specify a zone in the job / cluster config in the request or use [dataproc auto-zone placement](https://cloud.google.com/dataproc/docs/concepts/configuring-clusters/auto-zone) hence why regional endpoints for ultimately zonal resources. This means "unifying naming on location" will be less explicitly mapping to the API endpoint used for those resources that actually use the region endpoint and this might lead to confusing code like `submit_job(region=self.location,...)` 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: us...@infra.apache.org
[GitHub] [airflow] jaketf commented on issue #10687: Make region parameter required in Google Dataproc operators and hooks
jaketf commented on issue #10687: URL: https://github.com/apache/airflow/issues/10687#issuecomment-686848519 @turbaszek I do not think we should unify naming on location as this would not line up with the REST API or the python client lib. the right thing to do here is a bit confusing looking at the [REST API docs](https://cloud.google.com/dataproc/docs/reference/rest) Some dataproc resources (e.g. job / cluster) only have `regions/` endpoints. Other dataproc resources (workflow templates) have BOTH `regions/` and `locations/` endpoints. This is clearly intentional but I'm not entirely sure the distinction of when one would use `locations/` over the `regions/` endpoint. I imagine a user who understands this difference would want the flexibility to use either endpoint from their airflow DAGs and thus for hook methods / operators that interact with those resources we should provide mutually exclusive region AND location parameters. It's my hunch that this locations endpoint is a future proofing thing in case dataproc API allows you to submit jobs to clusters that aren't in GCP regions per say (this is purely my own speculation perhaps we can ask dataproc engineering team for this namespace distinction). Any given dataproc job runs only on a single dataproc cluster which will in only live in a single zone but we submit jobs to a regional endpoint and can OPTIONALLY specify a zone in the job / cluster config in the request or use [dataproc auto-zone placement](https://cloud.google.com/dataproc/docs/concepts/configuring-clusters/auto-zone) hence why regional endpoints for ultimately zonal resources. This means "unifying naming on location" will be less explicitly mapping to the API endpoint used for those resources that actually use the region endpoint and this might lead to confusing code like `submit_job(region=self.location,...)` 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: us...@infra.apache.org
[airflow] branch master updated (ab5235e -> 828f730)
This is an automated email from the ASF dual-hosted git repository. dimberman pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/airflow.git. from ab5235e Unify command names in CLI (#10720) add 828f730 Add generate_yaml command to easily test KubernetesExecutor before deploying pods (#10677) No new revisions were added by this update. Summary of changes: airflow/cli/cli_parser.py | 31 airflow/cli/commands/dag_command.py| 44 ++ docs/executor/kubernetes.rst | 3 +++ tests/cli/commands/test_dag_command.py | 12 ++ 4 files changed, 90 insertions(+)
[airflow] branch master updated (ab5235e -> 828f730)
This is an automated email from the ASF dual-hosted git repository. dimberman pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/airflow.git. from ab5235e Unify command names in CLI (#10720) add 828f730 Add generate_yaml command to easily test KubernetesExecutor before deploying pods (#10677) No new revisions were added by this update. Summary of changes: airflow/cli/cli_parser.py | 31 airflow/cli/commands/dag_command.py| 44 ++ docs/executor/kubernetes.rst | 3 +++ tests/cli/commands/test_dag_command.py | 12 ++ 4 files changed, 90 insertions(+)
[airflow] branch master updated (ab5235e -> 828f730)
This is an automated email from the ASF dual-hosted git repository. dimberman pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/airflow.git. from ab5235e Unify command names in CLI (#10720) add 828f730 Add generate_yaml command to easily test KubernetesExecutor before deploying pods (#10677) No new revisions were added by this update. Summary of changes: airflow/cli/cli_parser.py | 31 airflow/cli/commands/dag_command.py| 44 ++ docs/executor/kubernetes.rst | 3 +++ tests/cli/commands/test_dag_command.py | 12 ++ 4 files changed, 90 insertions(+)
[GitHub] [airflow] dimberman merged pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
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: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #9943: Increase typing coverage for postgres provider
mik-laj commented on pull request #9943: URL: https://github.com/apache/airflow/pull/9943#issuecomment-686826251 @kenjihiraoka Do you need any help? 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: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #9990: Relax requirement to allow latest version of flask-login
mik-laj commented on pull request #9990: URL: https://github.com/apache/airflow/pull/9990#issuecomment-686825244 @kaxil CI is sad. Can you look at it? I wanted to do a rebase, but I have no access right. ``` ERROR: Permission to astronomer/airflow.git denied to mik-laj. fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. ``` 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: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #10192: Deprecate BaseHook.get_connections method (#10135)
mik-laj commented on pull request #10192: URL: https://github.com/apache/airflow/pull/10192#issuecomment-686823492 > Do you think we can totally remove the use of get_connections before Airflow2.0? We should keep backward-compatibility with runtime warning. In Airlfow 2.1, we can remove all deprecated methods. 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: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #10256: fixes http hook using schema field from airflow.models.connection.Con…
mik-laj commented on pull request #10256: URL: https://github.com/apache/airflow/pull/10256#issuecomment-686822977 @kubatyszko Do you need help writing documentation about your findings? I am very happy to help with the review. 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: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #10261: 10166 : Add capability to specify gunicorn access log format for airflow
mik-laj commented on a change in pull request #10261: URL: https://github.com/apache/airflow/pull/10261#discussion_r483312664 ## File path: airflow/config_templates/config.yml ## @@ -861,6 +861,13 @@ type: string example: ~ default: "-" +- name: access_logformat + description: | +Access log format for gunicorn webserver Review comment: A link to documentation that describes possible format argument would be helpful. 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: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #10261: 10166 : Add capability to specify gunicorn access log format for airflow
mik-laj commented on a change in pull request #10261: URL: https://github.com/apache/airflow/pull/10261#discussion_r483312454 ## File path: airflow/config_templates/config.yml ## @@ -861,6 +861,13 @@ type: string example: ~ default: "-" +- name: access_logformat + description: | +Access log format for gunicorn webserver + version_added: ~ + type: string + example: ~ + default: "%%(h)s %%(l)s %%(u)s %%(t)s \"%%(r)s\" %%(s)s %%(b)s \"%%(f)s\" \"%%(a)s\"" Review comment: Should we define default value? If we pass an empty value, the default value for gunicorn will not be used? 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: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #10303: Add docs for how airflow manages packages and imports
mik-laj commented on pull request #10303: URL: https://github.com/apache/airflow/pull/10303#issuecomment-686821207 Awesome work! 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: us...@infra.apache.org
[GitHub] [airflow] breser commented on issue #10429: jquery dependency needs to be updated to 3.5.0 or newer
breser commented on issue #10429: URL: https://github.com/apache/airflow/issues/10429#issuecomment-686820980 See this comment on the PR I opened for past attempts to fix this: https://github.com/apache/airflow/pull/10684#issuecomment-686629969 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: us...@infra.apache.org
[GitHub] [airflow] breser commented on pull request #10684: Fix #10429 jquery dependency needs to be updated to 3.5.0 or newer
breser commented on pull request #10684: URL: https://github.com/apache/airflow/pull/10684#issuecomment-686820553 I'm closing this then because I have no time/ability to fix these deeper issues. 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: us...@infra.apache.org
[GitHub] [airflow] breser closed pull request #10684: Fix #10429 jquery dependency needs to be updated to 3.5.0 or newer
breser closed pull request #10684: URL: https://github.com/apache/airflow/pull/10684 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: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #10393: Simplify the K8sExecutor and K8sPodOperator
mik-laj commented on a change in pull request #10393:
URL: https://github.com/apache/airflow/pull/10393#discussion_r483310271
##
File path: tests/kubernetes/test_pod_generator.py
##
@@ -241,238 +213,224 @@ def test_gen_pod_extract_xcom(self, mock_uuid):
],
'resources': {'requests': {'cpu': '1m'}},
}
-self.expected['spec']['containers'].append(container_two)
-self.expected['spec']['containers'][0]['volumeMounts'].insert(0, {
-'name': 'xcom',
-'mountPath': '/airflow/xcom'
-})
-self.expected['spec']['volumes'].insert(0, {
-'name': 'xcom', 'emptyDir': {}
-})
-result_dict['spec']['containers'][0]['env'].sort(key=lambda x:
x['name'])
-self.assertEqual(result_dict, self.expected)
+self.expected.spec.containers.append(container_two)
+base_container: k8s.V1Container = self.expected.spec.containers[0]
+base_container.volume_mounts = base_container.volume_mounts or []
+base_container.volume_mounts.append(k8s.V1VolumeMount(
+name="xcom",
+mount_path="/airflow/xcom"
+))
+self.expected.spec.containers[0] = base_container
+self.expected.spec.volumes = self.expected.spec.volumes or []
+self.expected.spec.volumes.append(
+k8s.V1Volume(
+name='xcom',
+empty_dir={},
+)
+)
+result_dict = self.k8s_client.sanitize_for_serialization(result)
+expected_dict =
self.k8s_client.sanitize_for_serialization(self.expected)
+
+self.assertEqual(result_dict, expected_dict)
def test_from_obj(self):
-result = PodGenerator.from_obj({
-"KubernetesExecutor": {
-"annotations": {"test": "annotation"},
-"volumes": [
-{
-"name": "example-kubernetes-test-volume",
-"hostPath": {"path": "/tmp/"},
-},
-],
-"volume_mounts": [
-{
-"mountPath": "/foo/",
-"name": "example-kubernetes-test-volume",
-},
-],
+result = PodGenerator.from_obj(
+{
+"KubernetesExecutor": k8s.V1Pod(
+api_version="v1",
+kind="Pod",
+metadata=k8s.V1ObjectMeta(
+name="foo",
+annotations={"test": "annotation"}
+),
+spec=k8s.V1PodSpec(
+containers=[
+k8s.V1Container(
+name="base",
+volume_mounts=[
+k8s.V1VolumeMount(
+mount_path="/foo/",
+name="example-kubernetes-test-volume"
+)
+]
+)
+],
+volumes=[
+k8s.V1Volume(
+name="example-kubernetes-test-volume",
+host_path=k8s.V1HostPathVolumeSource(
+path="/tmp/"
+)
+)
+]
+)
+)
}
-})
+)
result = self.k8s_client.sanitize_for_serialization(result)
self.assertEqual({
'apiVersion': 'v1',
'kind': 'Pod',
'metadata': {
+'name': 'foo',
'annotations': {'test': 'annotation'},
},
'spec': {
'containers': [{
-'args': [],
-'command': [],
-'env': [],
-'envFrom': [],
'name': 'base',
-'ports': [],
'volumeMounts': [{
'mountPath': '/foo/',
'name': 'example-kubernetes-test-volume'
}],
}],
-'hostNetwork': False,
-'imagePullSecrets': [],
'volumes': [{
'hostPath': {'path': '/tmp/'},
'name': 'example-kubernetes-test-volume'
}],
}
}, result)
+# TODO: Should we save this feature?
+# result = PodGenerator.from_obj({
+# "KubernetesExecutor": {
+# "annotations": {"test": "annotation"},
+# "volumes": [
+# {
+# "name": "example-kubernetes-test-volume",
+#
[GitHub] [airflow] mik-laj commented on a change in pull request #10393: Simplify the K8sExecutor and K8sPodOperator
mik-laj commented on a change in pull request #10393:
URL: https://github.com/apache/airflow/pull/10393#discussion_r483310169
##
File path: tests/kubernetes/test_pod_generator.py
##
@@ -241,238 +213,224 @@ def test_gen_pod_extract_xcom(self, mock_uuid):
],
'resources': {'requests': {'cpu': '1m'}},
}
-self.expected['spec']['containers'].append(container_two)
-self.expected['spec']['containers'][0]['volumeMounts'].insert(0, {
-'name': 'xcom',
-'mountPath': '/airflow/xcom'
-})
-self.expected['spec']['volumes'].insert(0, {
-'name': 'xcom', 'emptyDir': {}
-})
-result_dict['spec']['containers'][0]['env'].sort(key=lambda x:
x['name'])
-self.assertEqual(result_dict, self.expected)
+self.expected.spec.containers.append(container_two)
+base_container: k8s.V1Container = self.expected.spec.containers[0]
+base_container.volume_mounts = base_container.volume_mounts or []
+base_container.volume_mounts.append(k8s.V1VolumeMount(
+name="xcom",
+mount_path="/airflow/xcom"
+))
+self.expected.spec.containers[0] = base_container
+self.expected.spec.volumes = self.expected.spec.volumes or []
+self.expected.spec.volumes.append(
+k8s.V1Volume(
+name='xcom',
+empty_dir={},
+)
+)
+result_dict = self.k8s_client.sanitize_for_serialization(result)
+expected_dict =
self.k8s_client.sanitize_for_serialization(self.expected)
+
+self.assertEqual(result_dict, expected_dict)
def test_from_obj(self):
-result = PodGenerator.from_obj({
-"KubernetesExecutor": {
-"annotations": {"test": "annotation"},
-"volumes": [
-{
-"name": "example-kubernetes-test-volume",
-"hostPath": {"path": "/tmp/"},
-},
-],
-"volume_mounts": [
-{
-"mountPath": "/foo/",
-"name": "example-kubernetes-test-volume",
-},
-],
+result = PodGenerator.from_obj(
+{
+"KubernetesExecutor": k8s.V1Pod(
+api_version="v1",
+kind="Pod",
+metadata=k8s.V1ObjectMeta(
+name="foo",
+annotations={"test": "annotation"}
+),
+spec=k8s.V1PodSpec(
+containers=[
+k8s.V1Container(
+name="base",
+volume_mounts=[
+k8s.V1VolumeMount(
+mount_path="/foo/",
+name="example-kubernetes-test-volume"
+)
+]
+)
+],
+volumes=[
+k8s.V1Volume(
+name="example-kubernetes-test-volume",
+host_path=k8s.V1HostPathVolumeSource(
+path="/tmp/"
+)
+)
+]
+)
+)
}
-})
+)
result = self.k8s_client.sanitize_for_serialization(result)
self.assertEqual({
'apiVersion': 'v1',
'kind': 'Pod',
'metadata': {
+'name': 'foo',
'annotations': {'test': 'annotation'},
},
'spec': {
'containers': [{
-'args': [],
-'command': [],
-'env': [],
-'envFrom': [],
'name': 'base',
-'ports': [],
'volumeMounts': [{
'mountPath': '/foo/',
'name': 'example-kubernetes-test-volume'
}],
}],
-'hostNetwork': False,
-'imagePullSecrets': [],
'volumes': [{
'hostPath': {'path': '/tmp/'},
'name': 'example-kubernetes-test-volume'
}],
}
}, result)
+# TODO: Should we save this feature?
Review comment:
?
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 servi
[GitHub] [airflow] mik-laj commented on pull request #10413: Add documentation for preparing database for Airflow
mik-laj commented on pull request #10413: URL: https://github.com/apache/airflow/pull/10413#issuecomment-686817728 @atsalolikhin-spokeo CI checks are sad. Can you fix it? Once you do this, I will be very happy to merge this PR. 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: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #10446: Add kubernetes connection type
mik-laj commented on pull request #10446: URL: https://github.com/apache/airflow/pull/10446#issuecomment-686817157 @turbaszek Be warned that this PR does not run all tests on CI. 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: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #10453: Allow to specify path to kubeconfig in KubernetesHook
mik-laj commented on a change in pull request #10453: URL: https://github.com/apache/airflow/pull/10453#discussion_r483307212 ## File path: airflow/providers/cncf/kubernetes/hooks/kubernetes.py ## @@ -36,32 +36,52 @@ class KubernetesHook(BaseHook): """ Creates Kubernetes API connection. +- use in cluster configuration by using `extra__kubernetes__in_cluster` in connection +- use custom configuration either by providing content of kubeconfig file via + `extra__kubernetes__kube_config` in connection +- use custom config by provideing path to the file using `extra__kubernetes__kube_config_path` +- use default config by providing no extras Review comment: ```suggestion - use in cluster configuration by using `extra__kubernetes__in_cluster` in connection - use custom configuration either by providing content of kubeconfig file via `extra__kubernetes__kube_config` in connection - use custom config by provideing path to the file using `extra__kubernetes__kube_config_path` - use default config by providing no extras ``` Can you update the guide also and add cross-reference between the hook and guide? https://airflow.readthedocs.io/en/latest/howto/connection/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: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #10514: Truly clear tis before runing dag test
mik-laj commented on pull request #10514: URL: https://github.com/apache/airflow/pull/10514#issuecomment-686815838 @turbaszek Be warned that this PR does not run all tests on CI. 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: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #10578: Add masterConfig parameter to MLEngineStartTrainingJobOperator
mik-laj commented on a change in pull request #10578:
URL: https://github.com/apache/airflow/pull/10578#discussion_r483305793
##
File path: tests/providers/google/cloud/operators/test_mlengine.py
##
@@ -350,6 +350,45 @@ def test_success_create_training_job(self, mock_hook):
project_id='test-project', job=self.TRAINING_INPUT,
use_existing_job_fn=ANY
)
+@patch('airflow.providers.google.cloud.operators.mlengine.MLEngineHook')
+def test_success_create_training_job_with_master_config(self, mock_hook):
+custom_training_default_args: dict =
copy.deepcopy(self.TRAINING_DEFAULT_ARGS)
+custom_training_default_args['scaleTier'] = 'CUSTOM'
Review comment:
```suggestion
```
I don't think this parameter exists.
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:
us...@infra.apache.org
[airflow] branch master updated (6f96e81 -> ab5235e)
This is an automated email from the ASF dual-hosted git repository. kamilbregula pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/airflow.git. from 6f96e81 Add Indeed to INTHEWILD.md (#10716) add ab5235e Unify command names in CLI (#10720) No new revisions were added by this update. Summary of changes: UPDATING.md| 1 + airflow/cli/cli_parser.py | 2 +- airflow/cli/commands/dag_command.py| 2 +- tests/cli/commands/test_dag_command.py | 6 +++--- 4 files changed, 6 insertions(+), 5 deletions(-)
[airflow] branch master updated (6f96e81 -> ab5235e)
This is an automated email from the ASF dual-hosted git repository. kamilbregula pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/airflow.git. from 6f96e81 Add Indeed to INTHEWILD.md (#10716) add ab5235e Unify command names in CLI (#10720) No new revisions were added by this update. Summary of changes: UPDATING.md| 1 + airflow/cli/cli_parser.py | 2 +- airflow/cli/commands/dag_command.py| 2 +- tests/cli/commands/test_dag_command.py | 6 +++--- 4 files changed, 6 insertions(+), 5 deletions(-)
[GitHub] [airflow] mik-laj merged pull request #10720: Unify command names in CLI
mik-laj merged pull request #10720: URL: 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: us...@infra.apache.org
[GitHub] [airflow] prakshalj0512 opened a new issue #10721: DAG Run ID referencing wrong DAG Run when trying to view
prakshalj0512 opened a new issue #10721: URL: https://github.com/apache/airflow/issues/10721 **Apache Airflow version**: 1.10.12 **Kubernetes version (if you are using kubernetes)** (use `kubectl version`): N/A **Environment**: - **Cloud provider or hardware configuration**: - **OS** (e.g. from /etc/os-release): MacOS & Linux - **Kernel** (e.g. `uname -a`): - **Install tools**: - **Others**: **What happened**: *I have set timezone=America/Phoenix in airflow.cfg.* When clicking on the DAG Run ID in the DAG list, it references the DAG run based on my timezone as opposed to the UTC one. Since all DAG Run IDs are comprised of UTC only, it ends up setting the Base Date to the wrong value and referencing the wrong DAG run. **What you expected to happen**: it should reference the correct DAG run. *What do you think went wrong?* When moving from DAG list to the individual DAG run view, it changes the UTC time to your timezone set in airflow.cfg and causes the mismatch. **How to reproduce it**: Set timezone to something other than UTC in airflow.cfg. Create a normal DAG; schedule it to execute every 1 min. Click on the Schedule button on the individual DAG view to get the DAG list. Click on one of the entries to view the DAG run. You should notice it refers to the wrong one. Screenshot Reference: https://user-images.githubusercontent.com/32403237/92182942-cbcfe980-ee01-11ea-900c-3e41e1d3299a.png";> https://user-images.githubusercontent.com/32403237/92182946-cd99ad00-ee01-11ea-86e0-cc365abb8476.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: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #10652: add securitySchemes in openapi spec
mik-laj commented on pull request #10652: URL: https://github.com/apache/airflow/pull/10652#issuecomment-686811482 @jhtimmins Can I ask for a review? 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: us...@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
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: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #10677: Add generate_yaml command to easily test KubernetesExecutor before deploying pods
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: us...@infra.apache.org
[GitHub] [airflow] houqp commented on a change in pull request #10594: WIP: Add permissions for stable API
houqp commented on a change in pull request #10594:
URL: https://github.com/apache/airflow/pull/10594#discussion_r483298085
##
File path: airflow/api_connexion/security.py
##
@@ -16,25 +16,61 @@
# under the License.
from functools import wraps
-from typing import Callable, TypeVar, cast
+from typing import Callable, Optional, Sequence, Tuple, TypeVar, cast
from flask import Response, current_app
-from airflow.api_connexion.exceptions import Unauthenticated
+from airflow.api_connexion.exceptions import PermissionDenied, Unauthenticated
T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name
-def requires_authentication(function: T):
-"""Decorator for functions that require authentication"""
+def check_authentication():
+"""Checks that the request has valid authorization information."""
+response = current_app.api_auth.requires_authentication(Response)()
+if response.status_code != 200:
+# since this handler only checks authentication, not authorization,
+# we should always return 401
+raise Unauthenticated(headers=response.headers)
-@wraps(function)
-def decorated(*args, **kwargs):
-response = current_app.api_auth.requires_authentication(Response)()
-if response.status_code != 200:
-# since this handler only checks authentication, not authorization,
-# we should always return 401
-raise Unauthenticated(headers=response.headers)
-return function(*args, **kwargs)
-return cast(T, decorated)
+def check_authorization(permissions=None, dag_id=None):
Review comment:
nitpick, could you add type annotations to this function?
##
File path: airflow/api_connexion/security.py
##
@@ -16,25 +16,61 @@
# under the License.
from functools import wraps
-from typing import Callable, TypeVar, cast
+from typing import Callable, Optional, Sequence, Tuple, TypeVar, cast
from flask import Response, current_app
-from airflow.api_connexion.exceptions import Unauthenticated
+from airflow.api_connexion.exceptions import PermissionDenied, Unauthenticated
T = TypeVar("T", bound=Callable) # pylint: disable=invalid-name
-def requires_authentication(function: T):
-"""Decorator for functions that require authentication"""
+def check_authentication():
+"""Checks that the request has valid authorization information."""
+response = current_app.api_auth.requires_authentication(Response)()
+if response.status_code != 200:
+# since this handler only checks authentication, not authorization,
+# we should always return 401
+raise Unauthenticated(headers=response.headers)
-@wraps(function)
-def decorated(*args, **kwargs):
-response = current_app.api_auth.requires_authentication(Response)()
-if response.status_code != 200:
-# since this handler only checks authentication, not authorization,
-# we should always return 401
-raise Unauthenticated(headers=response.headers)
-return function(*args, **kwargs)
-return cast(T, decorated)
+def check_authorization(permissions=None, dag_id=None):
+"""Checks that the logged in user has the specified permissions."""
+
+if not permissions:
+permissions = []
Review comment:
shouldn't we doing an early return here instead of creating an empty
permissions list?
##
File path: airflow/www/security.py
##
@@ -521,6 +524,17 @@ def sync_roles(self):
self.update_admin_perm_view()
self.clean_perms()
+def sync_resource_permissions(self, permissions=None):
+"""
+Populates resource-based permissions.
+"""
+if not permissions:
+permissions = []
Review comment:
same as my previous comment, seems like we can just do early return here.
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:
us...@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
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:
us...@infra.apache.org
[GitHub] [airflow] houqp commented on pull request #10351: Migrate speccy to spectral in OpenAPI linting.
houqp commented on pull request #10351: URL: https://github.com/apache/airflow/pull/10351#issuecomment-686806300 I think it's worth digging into the sibling issue as follow up. If it's a bug in openapi-generator, we should fix it from there, or maybe we can figure out a better way to mark objects as readonly. 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: us...@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
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 -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:
us...@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
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 -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:
us...@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
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 -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:
us...@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
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:
us...@infra.apache.org
[GitHub] [airflow] mik-laj opened a new pull request #10720: Unify command names in CLI
mik-laj opened a new pull request #10720: URL: https://github.com/apache/airflow/pull/10720 Part of: https://github.com/apache/airflow/pull/10669 --- **^ 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: us...@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
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 -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:
us...@infra.apache.org
[GitHub] [airflow] Labbs commented on issue #10292: Kubernetes Executor - invalid hostname in the database
Labbs commented on issue #10292: URL: https://github.com/apache/airflow/issues/10292#issuecomment-686801363 @jonstacks I tested your fix. That works well. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@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
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 -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:
us...@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
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 -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:
us...@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
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 -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:
us...@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
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 -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:
us...@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
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 -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:
us...@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
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 -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:
us...@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
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 -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:
us...@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
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:
us...@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
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:
us...@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
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:
us...@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
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.
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:
us...@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
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:
us...@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
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:
us...@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
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:
us...@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
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 -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:
us...@infra.apache.org
[GitHub] [airflow] dimberman commented on pull request #10666: Delete the pod launched by KubernetesPodOperator when task is cleared.
dimberman commented on pull request #10666: URL: https://github.com/apache/airflow/pull/10666#issuecomment-686786941 My pleasure @spencerschack :) 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: us...@infra.apache.org
[GitHub] [airflow] spencerschack commented on pull request #10666: Delete the pod launched by KubernetesPodOperator when task is cleared.
spencerschack commented on pull request #10666: URL: https://github.com/apache/airflow/pull/10666#issuecomment-686786777 Thanks @dimberman! Appreciate your hard work! 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: us...@infra.apache.org
[GitHub] [airflow] dimberman commented on pull request #10666: Delete the pod launched by KubernetesPodOperator when task is cleared.
dimberman commented on pull request #10666: URL: https://github.com/apache/airflow/pull/10666#issuecomment-686786315 @spencerschack oof good catch I'll make a quick PR to make that optional thank you. 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: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #10664: Validate values.yaml
mik-laj commented on pull request #10664: URL: https://github.com/apache/airflow/pull/10664#issuecomment-686785195 It would be fantastic if we could generate documentation for README.md based on this, but we should do it in a separate PR. It has a lot of potential. 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: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #10664: Validate values.yaml
mik-laj commented on a change in pull request #10664:
URL: https://github.com/apache/airflow/pull/10664#discussion_r483275190
##
File path: chart/values.schema.json
##
@@ -0,0 +1,1250 @@
+{
+"$schema": "http://json-schema.org/draft-07/schema";,
+"description": "Default values for airflow. Declare variables to be passed
into your templates.",
+"type": "object",
+"properties": {
+"uid": {
+"description": "User of airflow user.",
+"type": "integer"
+},
+"gid": {
+"description": "Group of airflow user.",
+"type": "integer"
+},
+"airflowHome": {
+"description": "Airflow home directory. Used for mount paths.",
+"type": "string"
+},
+"defaultAirflowRepository": {
+"description": "Default airflow repository. Overrides all the
specific images below.",
+"type": "string"
+},
+"defaultAirflowTag": {
+"description": "Default airflow tag to deploy.",
+"type": "string"
+},
+"nodeSelector": {
+"description": "Select certain nodes for airflow pods.",
+"type": "object"
+},
+"affinity": {
+"description": "Select certain nodes for airflow pods.",
+"type": "object"
+},
+"tolerations": {
+"description": "Select certain nodes for airflow pods.",
+"type": "array"
+},
+"labels": {
+"description": "Add common labels to all objects and pods defined
in this chart.",
+"type": "object"
Review comment:
```suggestion
"type": "object",
"additionalProperties": { "type": "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:
us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #10664: Validate values.yaml
mik-laj commented on a change in pull request #10664:
URL: https://github.com/apache/airflow/pull/10664#discussion_r483275331
##
File path: chart/values.schema.json
##
@@ -0,0 +1,1250 @@
+{
+"$schema": "http://json-schema.org/draft-07/schema";,
+"description": "Default values for airflow. Declare variables to be passed
into your templates.",
+"type": "object",
+"properties": {
+"uid": {
+"description": "User of airflow user.",
+"type": "integer"
+},
+"gid": {
+"description": "Group of airflow user.",
+"type": "integer"
+},
+"airflowHome": {
+"description": "Airflow home directory. Used for mount paths.",
+"type": "string"
+},
+"defaultAirflowRepository": {
+"description": "Default airflow repository. Overrides all the
specific images below.",
+"type": "string"
+},
+"defaultAirflowTag": {
+"description": "Default airflow tag to deploy.",
+"type": "string"
+},
+"nodeSelector": {
+"description": "Select certain nodes for airflow pods.",
+"type": "object"
Review comment:
```suggestion
"type": "object",
"additionalProperties": { "type": "string" }
```
? I'm not sure.
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:
us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #10664: Validate values.yaml
mik-laj commented on a change in pull request #10664:
URL: https://github.com/apache/airflow/pull/10664#discussion_r483274899
##
File path: chart/values.schema.json
##
@@ -0,0 +1,1250 @@
+{
+"$schema": "http://json-schema.org/draft-07/schema";,
+"description": "Default values for airflow. Declare variables to be passed
into your templates.",
+"type": "object",
+"properties": {
+"uid": {
+"description": "User of airflow user.",
+"type": "integer"
+},
+"gid": {
+"description": "Group of airflow user.",
+"type": "integer"
+},
+"airflowHome": {
+"description": "Airflow home directory. Used for mount paths.",
+"type": "string"
+},
+"defaultAirflowRepository": {
+"description": "Default airflow repository. Overrides all the
specific images below.",
+"type": "string"
+},
+"defaultAirflowTag": {
+"description": "Default airflow tag to deploy.",
+"type": "string"
+},
+"nodeSelector": {
+"description": "Select certain nodes for airflow pods.",
+"type": "object"
+},
+"affinity": {
+"description": "Select certain nodes for airflow pods.",
+"type": "object"
+},
+"tolerations": {
+"description": "Select certain nodes for airflow pods.",
+"type": "array"
+},
+"labels": {
+"description": "Add common labels to all objects and pods defined
in this chart.",
+"type": "object"
+},
+"ingress": {
+"description": "Ingress configuration.",
+"type": "object",
+"properties": {
+"enabled": {
+"description": "Enable ingress resource.",
+"type": "boolean"
+},
+"web": {
+"description": "Configuration for the Ingress of the web
Service.",
+"type": "object",
+"properties": {
+"annotations": {
+"description": "Annotations for the web Ingress.",
+"type": "object"
+},
+"path": {
+"description": "The path for the web Ingress.",
+"type": "string"
+},
+"host": {
+"description": "The hostname for the web Ingress.",
+"type": "string"
+},
+"tls": {
+"description": "Configuration for web Ingress
TLS.",
+"type": "object",
+"properties": {
+"enabled": {
+"description": "Enable TLS termination for
the web Ingress.",
+"type": "boolean"
+},
+"secretName": {
+"description": "The name of a pre-created
Secret containing a TLS private key and certificate.",
+"type": "string"
+}
+}
+},
+"precedingPaths": {
+"description": "HTTP paths to add to the web
Ingress before the default path.",
+"type": "array"
+},
+"succeedingPaths": {
+"description": "HTTP paths to add to the web
Ingress after the default path.",
+"type": "array"
+}
+}
+},
+"flower": {
+"description": "Configuration for the Ingress of the
flower Service.",
+"type": "object",
+"properties": {
+"annotations": {
+"description": "Annotations for the flower
Ingress.",
+"type": "object"
+},
+"path": {
+"description": "The path for the flower Ingress.",
+"type": "string"
+},
+"host": {
+"description": "The hostname for the flower
Ingress.",
+"type": "string"
+},
+"tls": {
+"description": "Configuration for flower Ingress
TLS.",
+"type": "object",
+"properties"
[GitHub] [airflow] mik-laj commented on a change in pull request #10664: Validate values.yaml
mik-laj commented on a change in pull request #10664:
URL: https://github.com/apache/airflow/pull/10664#discussion_r483274066
##
File path: chart/values.schema.json
##
@@ -0,0 +1,1250 @@
+{
+"$schema": "http://json-schema.org/draft-07/schema";,
+"description": "Default values for airflow. Declare variables to be passed
into your templates.",
+"type": "object",
+"properties": {
+"uid": {
+"description": "User of airflow user.",
+"type": "integer"
+},
+"gid": {
+"description": "Group of airflow user.",
+"type": "integer"
+},
+"airflowHome": {
+"description": "Airflow home directory. Used for mount paths.",
+"type": "string"
+},
+"defaultAirflowRepository": {
+"description": "Default airflow repository. Overrides all the
specific images below.",
+"type": "string"
+},
+"defaultAirflowTag": {
+"description": "Default airflow tag to deploy.",
+"type": "string"
+},
+"nodeSelector": {
+"description": "Select certain nodes for airflow pods.",
+"type": "object"
+},
+"affinity": {
+"description": "Select certain nodes for airflow pods.",
+"type": "object"
+},
+"tolerations": {
+"description": "Select certain nodes for airflow pods.",
+"type": "array"
+},
+"labels": {
+"description": "Add common labels to all objects and pods defined
in this chart.",
+"type": "object"
+},
+"ingress": {
+"description": "Ingress configuration.",
+"type": "object",
+"properties": {
+"enabled": {
+"description": "Enable ingress resource.",
+"type": "boolean"
+},
+"web": {
+"description": "Configuration for the Ingress of the web
Service.",
+"type": "object",
+"properties": {
+"annotations": {
+"description": "Annotations for the web Ingress.",
+"type": "object"
+},
+"path": {
+"description": "The path for the web Ingress.",
+"type": "string"
+},
+"host": {
+"description": "The hostname for the web Ingress.",
+"type": "string"
+},
+"tls": {
+"description": "Configuration for web Ingress
TLS.",
+"type": "object",
+"properties": {
+"enabled": {
+"description": "Enable TLS termination for
the web Ingress.",
+"type": "boolean"
+},
+"secretName": {
+"description": "The name of a pre-created
Secret containing a TLS private key and certificate.",
+"type": "string"
+}
+}
+},
+"precedingPaths": {
+"description": "HTTP paths to add to the web
Ingress before the default path.",
+"type": "array"
+},
+"succeedingPaths": {
+"description": "HTTP paths to add to the web
Ingress after the default path.",
+"type": "array"
+}
+}
+},
+"flower": {
+"description": "Configuration for the Ingress of the
flower Service.",
+"type": "object",
+"properties": {
+"annotations": {
+"description": "Annotations for the flower
Ingress.",
+"type": "object"
+},
+"path": {
+"description": "The path for the flower Ingress.",
+"type": "string"
+},
+"host": {
+"description": "The hostname for the flower
Ingress.",
+"type": "string"
+},
+"tls": {
+"description": "Configuration for flower Ingress
TLS.",
+"type": "object",
+"properties"
[GitHub] [airflow] jedcunningham opened a new pull request #10719: Include missing RBAC permission for extra links
jedcunningham opened a new pull request #10719: URL: https://github.com/apache/airflow/pull/10719 This change adds 'can extra links on Airflow' to the Viewer role and above. Currently, only Admins can see extra links 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: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #10664: Validate values.yaml
mik-laj commented on a change in pull request #10664:
URL: https://github.com/apache/airflow/pull/10664#discussion_r483272979
##
File path: chart/values.schema.json
##
@@ -0,0 +1,1250 @@
+{
+"$schema": "http://json-schema.org/draft-07/schema";,
+"description": "Default values for airflow. Declare variables to be passed
into your templates.",
+"type": "object",
+"properties": {
+"uid": {
+"description": "User of airflow user.",
+"type": "integer"
+},
+"gid": {
+"description": "Group of airflow user.",
+"type": "integer"
+},
+"airflowHome": {
+"description": "Airflow home directory. Used for mount paths.",
+"type": "string"
+},
+"defaultAirflowRepository": {
+"description": "Default airflow repository. Overrides all the
specific images below.",
+"type": "string"
+},
+"defaultAirflowTag": {
+"description": "Default airflow tag to deploy.",
+"type": "string"
+},
+"nodeSelector": {
+"description": "Select certain nodes for airflow pods.",
+"type": "object"
+},
+"affinity": {
+"description": "Select certain nodes for airflow pods.",
+"type": "object"
+},
+"tolerations": {
+"description": "Select certain nodes for airflow pods.",
+"type": "array"
+},
+"labels": {
+"description": "Add common labels to all objects and pods defined
in this chart.",
+"type": "object"
+},
+"ingress": {
+"description": "Ingress configuration.",
+"type": "object",
+"properties": {
+"enabled": {
+"description": "Enable ingress resource.",
+"type": "boolean"
+},
+"web": {
+"description": "Configuration for the Ingress of the web
Service.",
+"type": "object",
+"properties": {
+"annotations": {
+"description": "Annotations for the web Ingress.",
+"type": "object"
+},
+"path": {
+"description": "The path for the web Ingress.",
+"type": "string"
+},
+"host": {
+"description": "The hostname for the web Ingress.",
+"type": "string"
+},
+"tls": {
+"description": "Configuration for web Ingress
TLS.",
+"type": "object",
+"properties": {
+"enabled": {
+"description": "Enable TLS termination for
the web Ingress.",
+"type": "boolean"
+},
+"secretName": {
+"description": "The name of a pre-created
Secret containing a TLS private key and certificate.",
+"type": "string"
+}
+}
+},
+"precedingPaths": {
+"description": "HTTP paths to add to the web
Ingress before the default path.",
+"type": "array"
+},
+"succeedingPaths": {
+"description": "HTTP paths to add to the web
Ingress after the default path.",
+"type": "array"
+}
+}
+},
+"flower": {
+"description": "Configuration for the Ingress of the
flower Service.",
+"type": "object",
+"properties": {
+"annotations": {
+"description": "Annotations for the flower
Ingress.",
+"type": "object"
+},
+"path": {
+"description": "The path for the flower Ingress.",
+"type": "string"
+},
+"host": {
+"description": "The hostname for the flower
Ingress.",
+"type": "string"
+},
+"tls": {
+"description": "Configuration for flower Ingress
TLS.",
+"type": "object",
+"properties"
[GitHub] [airflow] mik-laj commented on a change in pull request #10664: Validate values.yaml
mik-laj commented on a change in pull request #10664:
URL: https://github.com/apache/airflow/pull/10664#discussion_r483272219
##
File path: chart/values.schema.json
##
@@ -0,0 +1,1250 @@
+{
+"$schema": "http://json-schema.org/draft-07/schema";,
+"description": "Default values for airflow. Declare variables to be passed
into your templates.",
+"type": "object",
+"properties": {
+"uid": {
+"description": "User of airflow user.",
+"type": "integer"
+},
+"gid": {
+"description": "Group of airflow user.",
+"type": "integer"
+},
+"airflowHome": {
+"description": "Airflow home directory. Used for mount paths.",
+"type": "string"
+},
+"defaultAirflowRepository": {
+"description": "Default airflow repository. Overrides all the
specific images below.",
+"type": "string"
+},
+"defaultAirflowTag": {
+"description": "Default airflow tag to deploy.",
+"type": "string"
+},
+"nodeSelector": {
+"description": "Select certain nodes for airflow pods.",
+"type": "object"
+},
+"affinity": {
+"description": "Select certain nodes for airflow pods.",
+"type": "object"
+},
+"tolerations": {
+"description": "Select certain nodes for airflow pods.",
+"type": "array"
+},
+"labels": {
+"description": "Add common labels to all objects and pods defined
in this chart.",
+"type": "object"
+},
+"ingress": {
+"description": "Ingress configuration.",
+"type": "object",
+"properties": {
+"enabled": {
+"description": "Enable ingress resource.",
+"type": "boolean"
+},
+"web": {
+"description": "Configuration for the Ingress of the web
Service.",
+"type": "object",
+"properties": {
+"annotations": {
+"description": "Annotations for the web Ingress.",
+"type": "object"
+},
+"path": {
+"description": "The path for the web Ingress.",
+"type": "string"
+},
+"host": {
+"description": "The hostname for the web Ingress.",
+"type": "string"
+},
+"tls": {
+"description": "Configuration for web Ingress
TLS.",
+"type": "object",
+"properties": {
+"enabled": {
+"description": "Enable TLS termination for
the web Ingress.",
+"type": "boolean"
+},
+"secretName": {
+"description": "The name of a pre-created
Secret containing a TLS private key and certificate.",
+"type": "string"
+}
+}
+},
+"precedingPaths": {
+"description": "HTTP paths to add to the web
Ingress before the default path.",
+"type": "array"
+},
+"succeedingPaths": {
+"description": "HTTP paths to add to the web
Ingress after the default path.",
+"type": "array"
+}
+}
+},
+"flower": {
+"description": "Configuration for the Ingress of the
flower Service.",
+"type": "object",
+"properties": {
+"annotations": {
+"description": "Annotations for the flower
Ingress.",
+"type": "object"
+},
+"path": {
+"description": "The path for the flower Ingress.",
+"type": "string"
+},
+"host": {
+"description": "The hostname for the flower
Ingress.",
+"type": "string"
+},
+"tls": {
+"description": "Configuration for flower Ingress
TLS.",
+"type": "object",
+"properties"
[jira] [Commented] (AIRFLOW-1424) Make the next execution date of DAGs visible
[ https://issues.apache.org/jira/browse/AIRFLOW-1424?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17190397#comment-17190397 ] ASF GitHub Bot commented on AIRFLOW-1424: - alexbegg edited a comment on pull request #2460: URL: https://github.com/apache/airflow/pull/2460#issuecomment-686774822 @ultrabug I am a bit too busy to look further into this today, but I will try and take a look this weekend. I have a WIP branch if you want to take a look. I am trying to add in your changes on top of the latest master, but _just for testing purposes_ I am trying to show the two new properties below the schedule link (currently both blank due to issues mentioned in my last comment), when I get it working I'll move the date to its final place as a tooltip. https://github.com/apache/airflow/compare/master...alexbegg:add-next-run 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: us...@infra.apache.org > Make the next execution date of DAGs visible > > > Key: AIRFLOW-1424 > URL: https://issues.apache.org/jira/browse/AIRFLOW-1424 > Project: Apache Airflow > Issue Type: New Feature > Components: DAG >Reporter: Ultrabug >Priority: Major > > The scheduler's DAG run creation logic can be tricky and one is > easily confused with the start_date + interval > and period end scheduling way of thinking. > It would ease airflow's usage to add a next execution field to DAGs > so that we can very easily see the (un)famous period end after which > the scheduler will create a new DAG run for our workflows. > These patches are a simple way to implement this on the DAG model > and make use of this in the interface. > Screenshots are on the github PR. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [airflow] alexbegg edited a comment on pull request #2460: [AIRFLOW-1424] make the next execution date of DAGs visible
alexbegg edited a comment on pull request #2460: URL: https://github.com/apache/airflow/pull/2460#issuecomment-686774822 @ultrabug I am a bit too busy to look further into this today, but I will try and take a look this weekend. I have a WIP branch if you want to take a look. I am trying to add in your changes on top of the latest master, but _just for testing purposes_ I am trying to show the two new properties below the schedule link (currently both blank due to issues mentioned in my last comment), when I get it working I'll move the date to its final place as a tooltip. https://github.com/apache/airflow/compare/master...alexbegg:add-next-run 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: us...@infra.apache.org
[jira] [Commented] (AIRFLOW-1424) Make the next execution date of DAGs visible
[ https://issues.apache.org/jira/browse/AIRFLOW-1424?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17190396#comment-17190396 ] ASF GitHub Bot commented on AIRFLOW-1424: - alexbegg commented on pull request #2460: URL: https://github.com/apache/airflow/pull/2460#issuecomment-686774822 @ultrabug I am a bit too busy to look further into this today, but I will try and take a look this weekend. I have a WIP branch if you want to take a look. I am trying to add in your changes on top of the latest master, but _just for testing purposes_ I am trying to show the two new properties below the schedule link (currently both blank due to issues mentioned in my last comment), when I get it working I'll move the date to its final place. https://github.com/apache/airflow/compare/master...alexbegg:add-next-run 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: us...@infra.apache.org > Make the next execution date of DAGs visible > > > Key: AIRFLOW-1424 > URL: https://issues.apache.org/jira/browse/AIRFLOW-1424 > Project: Apache Airflow > Issue Type: New Feature > Components: DAG >Reporter: Ultrabug >Priority: Major > > The scheduler's DAG run creation logic can be tricky and one is > easily confused with the start_date + interval > and period end scheduling way of thinking. > It would ease airflow's usage to add a next execution field to DAGs > so that we can very easily see the (un)famous period end after which > the scheduler will create a new DAG run for our workflows. > These patches are a simple way to implement this on the DAG model > and make use of this in the interface. > Screenshots are on the github PR. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [airflow] alexbegg commented on pull request #2460: [AIRFLOW-1424] make the next execution date of DAGs visible
alexbegg commented on pull request #2460: URL: https://github.com/apache/airflow/pull/2460#issuecomment-686774822 @ultrabug I am a bit too busy to look further into this today, but I will try and take a look this weekend. I have a WIP branch if you want to take a look. I am trying to add in your changes on top of the latest master, but _just for testing purposes_ I am trying to show the two new properties below the schedule link (currently both blank due to issues mentioned in my last comment), when I get it working I'll move the date to its final place. https://github.com/apache/airflow/compare/master...alexbegg:add-next-run 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: us...@infra.apache.org
[GitHub] [airflow] spencerschack commented on pull request #10666: Delete the pod launched by KubernetesPodOperator when task is cleared.
spencerschack commented on pull request #10666: URL: https://github.com/apache/airflow/pull/10666#issuecomment-686773180 @dimberman Sorry to just jump into the convo, but by passing 0 seconds as the grace period, you're telling k8s to send SIGKILL to the container process instead of SIGTERM and giving it a chance to clean up. This would be an incompatible change with my company's setup due to the fact that tasks need to clean up resources before they exit. At the very least the grace period should configurable. 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: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #10655: [AIRFLOW-10645] Add AWS Secrets Manager Hook
mik-laj commented on pull request #10655: URL: https://github.com/apache/airflow/pull/10655#issuecomment-686772675 @potiuk I know you wrote hooks for GCP Secret Manager and Hashicorp Vault. What do you think about this change? 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: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #10655: [AIRFLOW-10645] Add AWS Secrets Manager Hook
mik-laj commented on pull request #10655: URL: https://github.com/apache/airflow/pull/10655#issuecomment-686771887 @houqp I don't have experience with AWS. I am focusing on GCP. @feluelle Can I ask for a review? 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: us...@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
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: us...@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
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 -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:
us...@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
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:
us...@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
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:
us...@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
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. 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: us...@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
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 -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:
us...@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
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: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #10718: Switches to better BATS asserts
potiuk commented on pull request #10718: URL: https://github.com/apache/airflow/pull/10718#issuecomment-686763258 Others - with this one and few other changes implemented recently (and follow up tests)- our Bash scripts will become almost as good as structured python code :) 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: us...@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #10718: Switches to better BATS asserts
potiuk edited a comment on pull request #10718:
URL: https://github.com/apache/airflow/pull/10718#issuecomment-686762644
CC: @OmairK - I am also going to add more and more unit tests for the bash
functions of Breeze. This PR makes it super-easy to write and run new tests,
so you might want to give it a try :). BATS provides a very nice framework to
test bash functions - and in this PR I also added
https://github.com/bats-core/bats-assert and
https://github.com/bats-core/bats-support and
https://github.com/bats-core/bats-file which provide a really structured and
straightforward way to write asserts (assert output of a function, whether a
file exists, whether the function failed or succeeded and the like).
It's rather easy to write new tests:
* you add a ".bats" in `tests/bats_tests`.
* Then you write tests similar to:
```
setup() {
load bats_utils
# any common initiali`ztion goes here
}
teardown() {
# any common teardown goes here
}
@test "Test missing value for a parametert" {
export _BREEZE_ALLOWED_TEST_PARAMS="a b /c"
run parameters::check_and_save_allowed_param "TEST_PARAM" "Test Param"
"--message"
assert_output "
ERROR: Allowed Test Param: [ a b c ]. Is: ''.
Switch to supported value with --message flag."
assert_failure
}
```
the test above runs `parameters::check_and_save_allowed_param` without
TEST_PARAM value set and the method fails with the expected error 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:
us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #10718: Switches to better BATS asserts
potiuk commented on pull request #10718:
URL: https://github.com/apache/airflow/pull/10718#issuecomment-686762644
CC: @OmairK - I am also going to add more and more unit tests for the bash
functions of Breeze. This PR makes it super-easy to write and run new tests,
so you might want to give it a try :). BATS provides a very nice framework to
test bash functions - and in this PR I also added for
https://github.com/bats-core/bats-assert and
https://github.com/bats-core/bats-support and
https://github.com/bats-core/bats-file which provide a really structured and
straightforward way to write asserts (assert output of a function, whether a
file exists, whether the function failed or succeeded and the like).
It's rather easy to write new tests:
* you add a ".bats" in `tests/bats_tests`.
* Then you write tests similar to:
```
setup() {
load bats_utils
# any common initiali`ztion goes here
}
teardown() {
# any common teardown goes here
}
@test "Test missing value for a parametert" {
export _BREEZE_ALLOWED_TEST_PARAMS="a b /c"
run parameters::check_and_save_allowed_param "TEST_PARAM" "Test Param"
"--message"
assert_output "
ERROR: Allowed Test Param: [ a b c ]. Is: ''.
Switch to supported value with --message flag."
assert_failure
}
```
the test above runs `parameters::check_and_save_allowed_param` without
TEST_PARAM value set and the method fails with the expected error 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:
us...@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
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:
us...@infra.apache.org
