[GitHub] [airflow] shahar1 commented on pull request #27797: Add tests to PythonOperator
shahar1 commented on PR #27797: URL: https://github.com/apache/airflow/pull/27797#issuecomment-1345175705 > Still fixes needed :( I think the tests haven't passed until now because of other tests in the `main` branch. In that case, do we have to wait until the other tests are ok? @uranusjr Thank you for the fixes. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] shahar1 commented on pull request #27797: Add tests to PythonOperator
shahar1 commented on PR #27797: URL: https://github.com/apache/airflow/pull/27797#issuecomment-1345175665 > Still fixes needed :( I think the tests haven't passed until now because of other tests in the `main` branch. In that case, to merge this PR, do we have to wait until the other tests are ok along with this one? @uranusjr Thank you for the kind 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[airflow] branch main updated: KubernetesExecutor multi_namespace_mode can use namespace list to avoid requiring cluster role (#28047)
This is an automated email from the ASF dual-hosted git repository. xddeng pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/airflow.git The following commit(s) were added to refs/heads/main by this push: new c739a6a087 KubernetesExecutor multi_namespace_mode can use namespace list to avoid requiring cluster role (#28047) c739a6a087 is described below commit c739a6a08790423ba1e38464a15c4bac078277d3 Author: Xiaodong DENG AuthorDate: Fri Dec 9 22:51:00 2022 -0800 KubernetesExecutor multi_namespace_mode can use namespace list to avoid requiring cluster role (#28047) Co-authored-by: Daniel Standish <15932138+dstand...@users.noreply.github.com> Co-authored-by: potiuk --- airflow/config_templates/config.yml | 12 ++- airflow/config_templates/default_airflow.cfg | 8 +- airflow/executors/kubernetes_executor.py | 143 +-- airflow/kubernetes/kube_config.py| 8 ++ tests/executors/test_kubernetes_executor.py | 124 --- 5 files changed, 228 insertions(+), 67 deletions(-) diff --git a/airflow/config_templates/config.yml b/airflow/config_templates/config.yml index 8d99ec0470..2759eef2ff 100644 --- a/airflow/config_templates/config.yml +++ b/airflow/config_templates/config.yml @@ -2429,11 +2429,21 @@ - name: multi_namespace_mode description: | Allows users to launch pods in multiple namespaces. -Will require creating a cluster-role for the scheduler +Will require creating a cluster-role for the scheduler, +or use multi_namespace_mode_namespace_list configuration. version_added: 1.10.12 type: boolean example: ~ default: "False" +- name: multi_namespace_mode_namespace_list + description: | +If multi_namespace_mode is True while scheduler does not have a cluster-role, +give the list of namespaces where the scheduler will schedule jobs +Scheduler needs to have the necessary permissions in these namespaces. + version_added: 2.6.0 + type: string + example: ~ + default: "" - name: in_cluster description: | Use the service account kubernetes gives to pods to connect to kubernetes cluster. diff --git a/airflow/config_templates/default_airflow.cfg b/airflow/config_templates/default_airflow.cfg index 9d4e863c6e..365de48f50 100644 --- a/airflow/config_templates/default_airflow.cfg +++ b/airflow/config_templates/default_airflow.cfg @@ -1215,9 +1215,15 @@ delete_worker_pods_on_failure = False worker_pods_creation_batch_size = 1 # Allows users to launch pods in multiple namespaces. -# Will require creating a cluster-role for the scheduler +# Will require creating a cluster-role for the scheduler, +# or use multi_namespace_mode_namespace_list configuration. multi_namespace_mode = False +# If multi_namespace_mode is True while scheduler does not have a cluster-role, +# give the list of namespaces where the scheduler will schedule jobs +# Scheduler needs to have the necessary permissions in these namespaces. +multi_namespace_mode_namespace_list = + # Use the service account kubernetes gives to pods to connect to kubernetes cluster. # It's intended for clients that expect to be running inside a pod running on kubernetes. # It will raise an exception if called from a process not running in a kubernetes environment. diff --git a/airflow/executors/kubernetes_executor.py b/airflow/executors/kubernetes_executor.py index 788f1b3fec..d933a294e9 100644 --- a/airflow/executors/kubernetes_executor.py +++ b/airflow/executors/kubernetes_executor.py @@ -23,11 +23,11 @@ KubernetesExecutor. """ from __future__ import annotations -import functools import json import logging import multiprocessing import time +from collections import defaultdict from datetime import timedelta from queue import Empty, Queue from typing import TYPE_CHECKING, Any, Dict, Optional, Sequence, Tuple @@ -52,6 +52,8 @@ from airflow.utils.log.logging_mixin import LoggingMixin from airflow.utils.session import provide_session from airflow.utils.state import State +ALL_NAMESPACES = "ALL_NAMESPACES" + # TaskInstance key, command, configuration, pod_template_file KubernetesJobType = Tuple[TaskInstanceKey, CommandType, Any, Optional[str]] @@ -66,7 +68,7 @@ class ResourceVersion: """Singleton for tracking resourceVersion from Kubernetes.""" _instance = None -resource_version = "0" +resource_version: dict[str, str] = {} def __new__(cls): if cls._instance is None: @@ -79,8 +81,7 @@ class KubernetesJobWatcher(multiprocessing.Process, LoggingMixin): def __init__( self, -namespace: str | None, -multi_namespace_mode: bool, +namespace: str, watcher_queue: Queue[KubernetesWatchType], resource_version: str | None, scheduler_job_id: str, @@ -88,7 +89,6 @@ class
[GitHub] [airflow] XD-DENG merged pull request #28047: KubernetesExecutor multi_namespace_mode can use namespace list
XD-DENG merged PR #28047: URL: https://github.com/apache/airflow/pull/28047 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] XD-DENG commented on pull request #28047: KubernetesExecutor multi_namespace_mode can use namespace list
XD-DENG commented on PR #28047: URL: https://github.com/apache/airflow/pull/28047#issuecomment-1345156469 Thanks again @potiuk . The hint you pointed out was very useful. After adding https://github.com/apache/airflow/pull/28047/commits/cf8ce596e8fc82fec72b97860510fa4e908dd4d9, the CI is finally running successfully! (we just need to end the executor explicitly in the tests). There are three items we would like to further check later: - File Task Handler cannot read log properly from K8S tasks when it's `multi_namespace_mode` (it's designed in a way that it only handled one K8S namespace). It's a separate issue from this PR though. - @uranusjr suggested to further refactor `ResourceVersion` class, but agreed it can be done later as a separate PR. - @dstandish suggested we can consider using threads to manage multiple watchers. But for now I believe we are good to go ahead and merge this PR itself. I would like to have @dstandish and @potiuk as co-authors for your significant contribution to this PR, if you have no objection :-) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] blag opened a new issue, #28280: Inconsistent handling of newlines in tasks
blag opened a new issue, #28280: URL: https://github.com/apache/airflow/issues/28280 ### Apache Airflow version main (development) ### What happened Variables with newlines in them are not always reproduced exactly as they are pasted into the web UI. Depending on how they are rendered, Unix newlines (`\n`) are silently converted to Windows newlines (`\r\n`) when rendered. Additionally, when users try to specify `\r` CR/"carriage return" in Python code, that gets rendered as a `\n` LF/"line feed" character. Tested on both astro and Airflow with Breeze. ### What you think should happen instead Unix newlines (`\n`) should always be rendered as Unix newlines (`\n`); Windows newlines (`\r\n`) should always be rendered as Windows newlines. ### How to reproduce Steps to reproduce: 1. Spin up Airflow with Breeze (`breeze start-airflow`) - I tested with the PostgreSQL database backend 2. Install `dos2unix` within the Airflow container: `apt update; apt install --yes dos2unix` 3. Create a Variable in the Airflow UI named `newlines`, and ensure that its content contains newlines. Note that the non-newline content doesn't really matter. I used: ``` Line 1 Line 2 Line 3 ``` 4. Run this dag: ```python from datetime import datetime from airflow import DAG from airflow.operators.bash import BashOperator from airflow.operators.smooth import SmoothOperator with DAG("test_newlines", schedule=None) as dag: test_bash_operator_multiline_env_var = BashOperator( dag=dag, task_id="test_bash_operator_multiline_env_var", start_date=datetime.now(), env={ "MULTILINE_ENV_VAR": """Line 1 Line 2 Line 3""" }, append_env=True, bash_command=''' if [[ "$(echo $MULTILINE_ENV_VAR)" != "$(echo $MULTILINE_ENV_VAR | dos2unix)" ]]; then echo >&2 "Multiline environment variable contains newlines incorrectly converted to Windows CRLF" exit 1 fi''', ) test_bash_operator_heredoc_contains_newlines = BashOperator( dag=dag, task_id="test_bash_operator_heredoc_contains_newlines", start_date=datetime.now(), bash_command=""" diff <( cat <&2 "Bash heredoc contains newlines incorrectly converted to Windows CRLF" exit 1 } """, ) test_bash_operator_env_var_from_variable_jinja_interpolation = BashOperator( dag=dag, task_id="test_bash_operator_env_var_from_variable_jinja_interpolation", start_date=datetime.now(), env={ "ENV_VAR_AIRFLOW_VARIABLE_WITH_NEWLINES": "{{ var.value['newlines'] }}", }, append_env=True, bash_command=''' diff <(echo "$ENV_VAR_AIRFLOW_VARIABLE_WITH_NEWLINES") <(echo "$ENV_VAR_AIRFLOW_VARIABLE_WITH_NEWLINES" | dos2unix) || { echo >&2 "Environment variable contains newlines incorrectly converted to Windows CRLF" exit 1 } ''', ) test_bash_operator_from_variable_jinja_interpolation = BashOperator( dag=dag, task_id="test_bash_operator_from_variable_jinja_interpolation", start_date=datetime.now(), bash_command=''' diff <(echo "{{ var.value['newlines'] }}") <(echo "{{ var.value['newlines'] }}" | dos2unix) || { echo >&2 "Jinja interpolated string contains newlines incorrectly converted to Windows CRLF" exit 1 } ''', ) test_bash_operator_backslash_n_not_equals_backslash_r = BashOperator( dag=dag, task_id="test_bash_operator_backslash_n_not_equals_backslash_r", start_date=datetime.now(), bash_command=''' if [[ "\r" == "\n" ]]; then echo >&2 "Backslash-r has been incorrectly converted into backslash-n" exit 1 fi''', ) passing = SmoothOperator(dag=dag, task_id="passing", start_date=datetime.now()) failing = SmoothOperator(dag=dag, task_id="failing", start_date=datetime.now()) [ test_bash_operator_multiline_env_var, test_bash_operator_heredoc_contains_newlines, ] >> passing [ test_bash_operator_env_var_from_variable_jinja_interpolation, test_bash_operator_from_variable_jinja_interpolation, test_bash_operator_backslash_n_not_equals_backslash_r, ] >> failing ``` Warning: The `test_bash_operator_heredoc_contains_newlines` may not actually ever complete in Breeze. It does complete successfully when using `astro`.
[airflow] branch main updated (a6cda7cd23 -> d133ec75cf)
This is an automated email from the ASF dual-hosted git repository. dstandish pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/airflow.git from a6cda7cd23 Fix template rendering for Common SQL operators (#28202) add d133ec75cf Add is_local to CeleryKubernetesExecutor (#28277) No new revisions were added by this update. Summary of changes: airflow/executors/celery_kubernetes_executor.py| 2 ++ tests/executors/test_celery_kubernetes_executor.py | 3 +++ 2 files changed, 5 insertions(+)
[GitHub] [airflow] dstandish merged pull request #28277: Add is_local to CeleryKubernetesExecutor
dstandish merged PR #28277: URL: https://github.com/apache/airflow/pull/28277 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] quanglinh304 commented on issue #11514: Missing Task Instance StatsD metrics for KubernetesExecutor
quanglinh304 commented on issue #11514: URL: https://github.com/apache/airflow/issues/11514#issuecomment-1344985693 I'm a newbie in Airflow. I use CeleryExecutor for Airflow On the first day, it doesn't report airflow.ti_failures, airflow.ti_successes. But the next day, these metrics appear. I didn't set up, or config anything. I don't know why :( -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[airflow] branch constraints-main updated: Updating constraints. Build id:
This is an automated email from the ASF dual-hosted git repository. github-bot pushed a commit to branch constraints-main in repository https://gitbox.apache.org/repos/asf/airflow.git The following commit(s) were added to refs/heads/constraints-main by this push: new 1707bac828 Updating constraints. Build id: 1707bac828 is described below commit 1707bac8289046d878966257076bada91495acdc Author: Automated GitHub Actions commit AuthorDate: Sat Dec 10 00:13:52 2022 + Updating constraints. Build id: This update in constraints is automatically committed by the CI 'constraints-push' step based on HEAD of '' in '' with commit sha . All tests passed in this build so we determined we can push the updated constraints. See https://github.com/apache/airflow/blob/main/README.md#installing-from-pypi for details. --- constraints-3.10.txt | 8 constraints-3.7.txt | 8 constraints-3.8.txt | 8 constraints-3.9.txt | 8 constraints-no-providers-3.10.txt | 2 +- constraints-no-providers-3.7.txt | 2 +- constraints-no-providers-3.8.txt | 2 +- constraints-no-providers-3.9.txt | 2 +- constraints-source-providers-3.10.txt | 8 constraints-source-providers-3.7.txt | 8 constraints-source-providers-3.8.txt | 8 constraints-source-providers-3.9.txt | 8 12 files changed, 36 insertions(+), 36 deletions(-) diff --git a/constraints-3.10.txt b/constraints-3.10.txt index 530c7b816b..a706dbe5b1 100644 --- a/constraints-3.10.txt +++ b/constraints-3.10.txt @@ -1,5 +1,5 @@ # -# This constraints file was automatically generated on 2022-12-09T20:13:29Z +# This constraints file was automatically generated on 2022-12-10T00:13:10Z # via "eager-upgrade" mechanism of PIP. For the "main" branch of Airflow. # This variant of constraints install uses the HEAD of the branch version for 'apache-airflow' but installs # the providers from PIP-released packages at the moment of the constraint generation. @@ -173,9 +173,9 @@ billiard==3.6.4.0 black==22.12.0 bleach==5.0.1 blinker==1.5 -boto3==1.26.26 +boto3==1.26.27 boto==2.49.0 -botocore==1.29.26 +botocore==1.29.27 bowler==0.9.0 cachelib==0.9.0 cachetools==4.2.2 @@ -532,7 +532,7 @@ snakebite-py3==3.0.5 sniffio==1.3.0 snowballstemmer==2.2.0 snowflake-connector-python==2.8.3 -snowflake-sqlalchemy==1.4.4 +snowflake-sqlalchemy==1.4.5 sortedcontainers==2.4.0 soupsieve==2.3.2.post1 sphinx-airflow-theme==0.0.11 diff --git a/constraints-3.7.txt b/constraints-3.7.txt index 5c11f909f3..00e5e00cd9 100644 --- a/constraints-3.7.txt +++ b/constraints-3.7.txt @@ -1,5 +1,5 @@ # -# This constraints file was automatically generated on 2022-12-09T20:13:47Z +# This constraints file was automatically generated on 2022-12-10T00:13:49Z # via "eager-upgrade" mechanism of PIP. For the "main" branch of Airflow. # This variant of constraints install uses the HEAD of the branch version for 'apache-airflow' but installs # the providers from PIP-released packages at the moment of the constraint generation. @@ -173,9 +173,9 @@ billiard==3.6.4.0 black==22.12.0 bleach==5.0.1 blinker==1.5 -boto3==1.26.26 +boto3==1.26.27 boto==2.49.0 -botocore==1.29.26 +botocore==1.29.27 bowler==0.9.0 cached-property==1.5.2 cachelib==0.9.0 @@ -533,7 +533,7 @@ snakebite-py3==3.0.5 sniffio==1.3.0 snowballstemmer==2.2.0 snowflake-connector-python==2.8.3 -snowflake-sqlalchemy==1.4.4 +snowflake-sqlalchemy==1.4.5 sortedcontainers==2.4.0 soupsieve==2.3.2.post1 sphinx-airflow-theme==0.0.11 diff --git a/constraints-3.8.txt b/constraints-3.8.txt index 60e1322f72..5e7446301a 100644 --- a/constraints-3.8.txt +++ b/constraints-3.8.txt @@ -1,5 +1,5 @@ # -# This constraints file was automatically generated on 2022-12-09T20:13:41Z +# This constraints file was automatically generated on 2022-12-10T00:13:39Z # via "eager-upgrade" mechanism of PIP. For the "main" branch of Airflow. # This variant of constraints install uses the HEAD of the branch version for 'apache-airflow' but installs # the providers from PIP-released packages at the moment of the constraint generation. @@ -174,9 +174,9 @@ billiard==3.6.4.0 black==22.12.0 bleach==5.0.1 blinker==1.5 -boto3==1.26.26 +boto3==1.26.27 boto==2.49.0 -botocore==1.29.26 +botocore==1.29.27 bowler==0.9.0 cachelib==0.9.0 cachetools==4.2.2 @@ -535,7 +535,7 @@ snakebite-py3==3.0.5 sniffio==1.3.0 snowballstemmer==2.2.0 snowflake-connector-python==2.8.3 -snowflake-sqlalchemy==1.4.4 +snowflake-sqlalchemy==1.4.5 sortedcontainers==2.4.0 soupsieve==2.3.2.post1 sphinx-airflow-theme==0.0.11 diff --git a/constraints-3.9.txt b/constraints-3.9.txt index cc674a4d65..73eef8830f 100644 --- a/constraints-3.9.txt +++ b/constraints-3.9.txt @@ -1,5 +1,5 @@ # -# This constraints file was automatically generated on 2022-12-09T20:13:38Z +# This constraints file was automatically
[GitHub] [airflow] o-nikolas commented on a diff in pull request #28161: WIP AIP-51 - Executor Coupling in Logging
o-nikolas commented on code in PR #28161: URL: https://github.com/apache/airflow/pull/28161#discussion_r1044920796 ## airflow/utils/log/file_task_handler.py: ## @@ -190,74 +220,22 @@ def _read(self, ti: TaskInstance, try_number: int, metadata: dict[str, Any] | No log = f"*** Failed to load local log file: {location}\n" log += f"*** {str(e)}\n" return log, {"end_of_log": True} -elif self._should_check_k8s(ti.queue): -try: -from airflow.kubernetes.kube_client import get_kube_client - -kube_client = get_kube_client() +else: -log += f"*** Trying to get logs (last 100 lines) from worker pod {ti.hostname} ***\n\n" +log += f"*** Log file does not exist: {location}\n" +executor = ExecutorLoader.get_default_executor() Review Comment: You may need to first check if that method exists on the executor class before calling it or update some of the other executors, pending the result of the discussion here: https://github.com/apache/airflow/issues/28276#issuecomment-1344899475 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] o-nikolas commented on a diff in pull request #28161: WIP AIP-51 - Executor Coupling in Logging
o-nikolas commented on code in PR #28161: URL: https://github.com/apache/airflow/pull/28161#discussion_r1044914857 ## airflow/utils/log/file_task_handler.py: ## @@ -190,74 +220,22 @@ def _read(self, ti: TaskInstance, try_number: int, metadata: dict[str, Any] | No log = f"*** Failed to load local log file: {location}\n" log += f"*** {str(e)}\n" return log, {"end_of_log": True} -elif self._should_check_k8s(ti.queue): -try: -from airflow.kubernetes.kube_client import get_kube_client - -kube_client = get_kube_client() +else: -log += f"*** Trying to get logs (last 100 lines) from worker pod {ti.hostname} ***\n\n" +log += f"*** Log file does not exist: {location}\n" Review Comment: ```suggestion log += f"*** Local log file does not exist, trying to fetch logs from executor environment ***\n\n" ``` This more closely matches what was there previously as well as the new context you added. ## airflow/utils/log/file_task_handler.py: ## @@ -190,74 +220,22 @@ def _read(self, ti: TaskInstance, try_number: int, metadata: dict[str, Any] | No log = f"*** Failed to load local log file: {location}\n" log += f"*** {str(e)}\n" return log, {"end_of_log": True} -elif self._should_check_k8s(ti.queue): -try: -from airflow.kubernetes.kube_client import get_kube_client - -kube_client = get_kube_client() +else: -log += f"*** Trying to get logs (last 100 lines) from worker pod {ti.hostname} ***\n\n" +log += f"*** Log file does not exist: {location}\n" +executor = ExecutorLoader.get_default_executor() Review Comment: You may need to first check if that method exists on the executor class before calling it, pending the result of the discussion here: https://github.com/apache/airflow/issues/28276#issuecomment-1344899475 ## tests/utils/test_log_handlers.py: ## @@ -267,36 +264,3 @@ def test_log_retrieval_valid(self, create_task_instance): log_url_ti.hostname = "hostname" url = FileTaskHandler._get_log_retrieval_url(log_url_ti, "DYNAMIC_PATH") assert url == "http://hostname:8793/log/DYNAMIC_PATH; - - -@pytest.mark.parametrize( -"config, queue, expected", -[ -(dict(AIRFLOW__CORE__EXECUTOR="LocalExecutor"), None, False), -(dict(AIRFLOW__CORE__EXECUTOR="LocalExecutor"), "kubernetes", False), -(dict(AIRFLOW__CORE__EXECUTOR="KubernetesExecutor"), None, True), -(dict(AIRFLOW__CORE__EXECUTOR="CeleryKubernetesExecutor"), "any", False), -(dict(AIRFLOW__CORE__EXECUTOR="CeleryKubernetesExecutor"), "kubernetes", True), -( -dict( -AIRFLOW__CORE__EXECUTOR="CeleryKubernetesExecutor", - AIRFLOW__CELERY_KUBERNETES_EXECUTOR__KUBERNETES_QUEUE="hithere", -), -"hithere", -True, -), -(dict(AIRFLOW__CORE__EXECUTOR="LocalKubernetesExecutor"), "any", False), -(dict(AIRFLOW__CORE__EXECUTOR="LocalKubernetesExecutor"), "kubernetes", True), -( -dict( -AIRFLOW__CORE__EXECUTOR="LocalKubernetesExecutor", -AIRFLOW__LOCAL_KUBERNETES_EXECUTOR__KUBERNETES_QUEUE="hithere", -), -"hithere", -True, -), -], -) -def test__should_check_k8s(config, queue, expected): -with patch.dict("os.environ", **config): -assert FileTaskHandler._should_check_k8s(queue) == expected Review Comment: You've nicely refactored `FileTaskHandler._read` to be unittestable. You can mock `os.path.exists(location)` to return false and also mock the kubernetes executor, then ensure `get_task_log` was called once with the expected ti input. You should then swap the executor to one that doesn't have an implementation and ensure you get None back (you shouldn't need to mock in that case since it has no implementation) and that the `_get_task_log_from_worker` method is called once (will need to mock that one). ## airflow/utils/log/file_task_handler.py: ## @@ -190,74 +220,22 @@ def _read(self, ti: TaskInstance, try_number: int, metadata: dict[str, Any] | No log = f"*** Failed to load local log file: {location}\n" log += f"*** {str(e)}\n" return log, {"end_of_log": True} -elif self._should_check_k8s(ti.queue): -try: -from airflow.kubernetes.kube_client import get_kube_client - -kube_client = get_kube_client() +else: -log += f"*** Trying to get logs (last 100 lines) from worker pod {ti.hostname} ***\n\n" +log += f"*** Log file does not exist: {location}\n" +
[GitHub] [airflow] github-actions[bot] commented on pull request #23560: Add advanced secrets backend configurations
github-actions[bot] commented on PR #23560: URL: https://github.com/apache/airflow/pull/23560#issuecomment-1344925482 This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] o-nikolas commented on issue #28276: AIP - 51 Make CeleryKubernetesExecutor extends BaseExecutor for a common API.
o-nikolas commented on issue #28276: URL: https://github.com/apache/airflow/issues/28276#issuecomment-1344899475 > Should I leave this open until your proposal come through, just to keep track of this specificity ? Maybe I can remove the AIP-51 if this is considered out of scope ? We'll still need some short term solution which this ticket can capture? At least to fix what's been released so far (which I think are only the `is_local` and `supports_ad_hoc_ti_run` fields) and then whatever we decide here will need to be done for future PRs too. Two approaches I see: 1. The fields that have been added/modified so far (e.g. `is_local` and `supports_ad_hoc_ti_run`) can be added directly to the executors which don't inherit from the base executor (e.g. `CeleryKubernetesExecutor`, `LocalKubernetesExecutor`, etc) 1. Pros: fewer code changes; encapsulates the fix to code that will eventually be deprecated/heavily modified. 2. Cons: only works for executors within Airflow core. Any custom 3rd party executors which don't inherit from BaseExecutor will also break once this code is released (**what level of support and backwards compatibility do we owe these 3rd party executors?**) 1. All the locations which use the new fields added to the baseExecutor (e.g. `is_local` and `supports_ad_hoc_ti_run`) should first check if the executor object has that attribute set, and if not, use the default from the BaseExceutor class (this is approximately how `supports_ad_hoc_ti_run` was originally implemented) 1. Pros: Will be backwards compatible for all executors both in Airflow core and public/3rd party executors 2. Cons: much uglier code and more brittle to maintain and eventually deprecate WDYT @pierrejeambrun? (also cc: @potiuk @eladkal) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] Taragolis opened a new pull request, #28279: Add Amazon Elastic Container Registry (ECR) Hook
Taragolis opened a new pull request, #28279: URL: https://github.com/apache/airflow/pull/28279 Pre-requirements for https://github.com/apache/airflow/pull/26162 Add new Hook into amazon-provider which integrated with ECR. Hook itself it is just Thin wrapper around boto3 ECR client with single method for: - Obtain temporary credentials - Decode them - Mask password - Prepare to use with `docker login` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] pierrejeambrun commented on issue #28276: AIP - 51 Make CeleryKubernetesExecutor extends BaseExecutor for a common API.
pierrejeambrun commented on issue #28276: URL: https://github.com/apache/airflow/issues/28276#issuecomment-1344891102 @o-nikolas, Even better, can't wait! Should I leave this open until your proposal come through, just to keep track of it problem ? Maybe I can remove the AIP-51 if this is considered out of scope ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] j-martin commented on issue #24526: upgrading from 2.2.3 or 2.2.5 to 2.3.2 fails on migration-job
j-martin commented on issue #24526: URL: https://github.com/apache/airflow/issues/24526#issuecomment-1344890185 For posterity and in case it helps somebody else we fix our schema with the queries below. Note that it assumes that the migrations from 2.2 to 2.3 have not been executed and failed. Since DDL changes are not transaction in MySQL (or at least the way Alembic is configured), the state of the database will be different after a failed migration. ```sql ALTER DATABASE airflow CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci; ALTER TABLE dag_tag DROP FOREIGN KEY dag_tag_ibfk_1; ALTER TABLE dag_tag MODIFY dag_id VARCHAR(250) CHARACTER SET utf8mb3 COLLATE utf8mb3_bin; ALTER TABLE dag MODIFY dag_id VARCHAR(250) CHARACTER SET utf8mb3 COLLATE utf8mb3_bin; ALTER TABLE dag_tag ADD CONSTRAINT dag_tag_ibfk_1 FOREIGN KEY (dag_id) REFERENCES dag (dag_id) ON DELETE CASCADE; ALTER TABLE task_fail MODIFY dag_id VARCHAR(250) CHARACTER SET utf8mb3 COLLATE utf8mb3_bin; ALTER TABLE job MODIFY dag_id VARCHAR(250) CHARACTER SET utf8mb3 COLLATE utf8mb3_bin; ALTER TABLE log MODIFY dag_id VARCHAR(250) CHARACTER SET utf8mb3 COLLATE utf8mb3_bin; ALTER TABLE sensor_instance MODIFY dag_id VARCHAR(250) CHARACTER SET utf8mb3 COLLATE utf8mb3_bin; ALTER TABLE serialized_dag MODIFY dag_id VARCHAR(250) CHARACTER SET utf8mb3 COLLATE utf8mb3_bin; ALTER TABLE sla_miss MODIFY dag_id VARCHAR(250) CHARACTER SET utf8mb3 COLLATE utf8mb3_bin; alter table task_instance drop foreign key task_instance_dag_run_fkey; ALTER TABLE task_reschedule DROP FOREIGN KEY task_reschedule_ti_fkey; ALTER TABLE task_reschedule MODIFY task_id VARCHAR(255) CHARACTER SET utf8mb3 COLLATE utf8mb3_bin; ALTER TABLE task_instance MODIFY dag_id VARCHAR(250) CHARACTER SET utf8mb3 COLLATE utf8mb3_bin; alter table task_instance add constraint task_instance_dag_run_fkey foreign key (dag_id, run_id) references dag_run (dag_id, run_id) on delete cascade; ALTER TABLE task_reschedule MODIFY dag_id VARCHAR(250) CHARACTER SET utf8mb3 COLLATE utf8mb3_bin; ALTER TABLE xcom MODIFY dag_id VARCHAR(250) CHARACTER SET utf8mb3 COLLATE utf8mb3_bin; ALTER TABLE task_instance MODIFY task_id VARCHAR(255) CHARACTER SET utf8mb3 COLLATE utf8mb3_bin; ALTER TABLE task_reschedule ADD CONSTRAINT task_reschedule_ti_fkey FOREIGN KEY (dag_id, task_id, run_id) REFERENCES task_instance (dag_id, task_id, run_id); ALTER TABLE rendered_task_instance_fields MODIFY dag_id VARCHAR(255) CHARACTER SET utf8mb3 COLLATE utf8mb3_bin; ALTER TABLE rendered_task_instance_fields MODIFY task_id VARCHAR(255) CHARACTER SET utf8mb3 COLLATE utf8mb3_bin; ALTER TABLE task_fail MODIFY task_id VARCHAR(255) CHARACTER SET utf8mb3 COLLATE utf8mb3_bin; ALTER TABLE task_fail MODIFY dag_id VARCHAR(255) CHARACTER SET utf8mb3 COLLATE utf8mb3_bin; ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28254: Proper Python Host output from composite tasks in CI
potiuk commented on PR #28254: URL: https://github.com/apache/airflow/pull/28254#issuecomment-1344882654 OK. All looks good, cache is solved. Ready to merge. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[airflow] branch main updated: Fix template rendering for Common SQL operators (#28202)
This is an automated email from the ASF dual-hosted git repository. potiuk pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/airflow.git The following commit(s) were added to refs/heads/main by this push: new a6cda7cd23 Fix template rendering for Common SQL operators (#28202) a6cda7cd23 is described below commit a6cda7cd230ef22f7fe042d6d5e9f78c660c4a75 Author: Jonathan Stott <81567861+jon-evergr...@users.noreply.github.com> AuthorDate: Fri Dec 9 23:04:38 2022 + Fix template rendering for Common SQL operators (#28202) Closes: #28195 This patch fixes all the common SQL operators I could find which showed the same problem as reported in #28195, that statements are generated "too early", before the templated variables have been applied. I think all changes should have tests which break without the fix. Some of these tests are quite brittle in that they mock complex nested SQL which is not ideal. This patch adds the `self.sql` variable as a templated parameter, allowing for templated `table`, `partition_clause`, `checks` etc. --- airflow/providers/common/sql/operators/sql.py| 28 +++-- airflow/providers/common/sql/operators/sql.pyi | 2 + tests/providers/common/sql/operators/test_sql.py | 134 +++ 3 files changed, 154 insertions(+), 10 deletions(-) diff --git a/airflow/providers/common/sql/operators/sql.py b/airflow/providers/common/sql/operators/sql.py index 09034b104c..cc2e9e4b57 100644 --- a/airflow/providers/common/sql/operators/sql.py +++ b/airflow/providers/common/sql/operators/sql.py @@ -324,7 +324,8 @@ class SQLColumnCheckOperator(BaseSQLOperator): :ref:`howto/operator:SQLColumnCheckOperator` """ -template_fields = ("partition_clause",) +template_fields = ("partition_clause", "table", "sql") +template_fields_renderers = {"sql": "sql"} sql_check_template = """ SELECT '{column}' AS col_name, '{check}' AS check_type, {column}_{check} AS check_result @@ -550,7 +551,9 @@ class SQLTableCheckOperator(BaseSQLOperator): :ref:`howto/operator:SQLTableCheckOperator` """ -template_fields = ("partition_clause",) +template_fields = ("partition_clause", "table", "sql") + +template_fields_renderers = {"sql": "sql"} sql_check_template = """ SELECT '{check_name}' AS check_name, MIN({check_name}) AS check_result @@ -603,6 +606,8 @@ class SQLTableCheckOperator(BaseSQLOperator): self.log.info("All tests have passed") def _generate_sql_query(self): +self.log.info("Partition clause: %s", self.partition_clause) + def _generate_partition_clause(check_name): if self.partition_clause and "partition_clause" not in self.checks[check_name]: return f"WHERE {self.partition_clause}" @@ -953,8 +958,8 @@ class SQLThresholdCheckOperator(BaseSQLOperator): ): super().__init__(conn_id=conn_id, database=database, **kwargs) self.sql = sql -self.min_threshold = _convert_to_float_if_possible(min_threshold) -self.max_threshold = _convert_to_float_if_possible(max_threshold) +self.min_threshold = min_threshold +self.max_threshold = max_threshold def execute(self, context: Context): hook = self.get_db_hook() @@ -962,15 +967,18 @@ class SQLThresholdCheckOperator(BaseSQLOperator): if not result: self._raise_exception(f"The following query returned zero rows: {self.sql}") -if isinstance(self.min_threshold, float): -lower_bound = self.min_threshold +min_threshold = _convert_to_float_if_possible(self.min_threshold) +max_threshold = _convert_to_float_if_possible(self.max_threshold) + +if isinstance(min_threshold, float): +lower_bound = min_threshold else: -lower_bound = hook.get_first(self.min_threshold)[0] +lower_bound = hook.get_first(min_threshold)[0] -if isinstance(self.max_threshold, float): -upper_bound = self.max_threshold +if isinstance(max_threshold, float): +upper_bound = max_threshold else: -upper_bound = hook.get_first(self.max_threshold)[0] +upper_bound = hook.get_first(max_threshold)[0] meta_data = { "result": result, diff --git a/airflow/providers/common/sql/operators/sql.pyi b/airflow/providers/common/sql/operators/sql.pyi index cbbd8ddcdc..70e24ce240 100644 --- a/airflow/providers/common/sql/operators/sql.pyi +++ b/airflow/providers/common/sql/operators/sql.pyi @@ -80,6 +80,7 @@ class SQLExecuteQueryOperator(BaseSQLOperator): class SQLColumnCheckOperator(BaseSQLOperator): template_fields: Incomplete +template_fields_renderers: Incomplete sql_check_template: str column_checks: Incomplete table: Incomplete @@ -102,6 +103,7 @@ class
[GitHub] [airflow] boring-cyborg[bot] commented on pull request #28202: Fix template rendering for Common SQL operators
boring-cyborg[bot] commented on PR #28202: URL: https://github.com/apache/airflow/pull/28202#issuecomment-1344869409 Awesome work, congrats on your first merged pull request! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #28202: Fix template rendering for Common SQL operators
potiuk merged PR #28202: URL: https://github.com/apache/airflow/pull/28202 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk closed issue #28195: SQLTableCheckOperator doesn't correctly handle templated partition clause
potiuk closed issue #28195: SQLTableCheckOperator doesn't correctly handle templated partition clause URL: https://github.com/apache/airflow/issues/28195 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #28275: Weird behaviour of Executor Queues leading to Quarantined test_impersonation_subdag in CI
potiuk commented on issue #28275: URL: https://github.com/apache/airflow/issues/28275#issuecomment-1344869128 cc: @Taragolis too. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #28275: Weird behaviour of Executor Queues leading to Quarantined test_impersonation_subdag in CI
potiuk commented on issue #28275: URL: https://github.com/apache/airflow/issues/28275#issuecomment-1344864771 Same behaviour here https://github.com/apache/airflow/actions/runs/3660108509/jobs/6187115621 - so it's not as rare. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk closed pull request #28255: Revert "Unquarantine background webserver test (#28249)"
potiuk closed pull request #28255: Revert "Unquarantine background webserver test (#28249)" URL: https://github.com/apache/airflow/pull/28255 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28255: Revert "Unquarantine background webserver test (#28249)"
potiuk commented on PR #28255: URL: https://github.com/apache/airflow/pull/28255#issuecomment-1344864039 Fingers crossed but seems that https://github.com/apache/airflow/pull/28252 fixed the problem. 爛 3 full "green" main/canary builds since it was merged - all of them fully green. Closing. Thanks @Taragolis for the fix. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[airflow] branch main updated (171ca66142 -> aace30b50c)
This is an automated email from the ASF dual-hosted git repository. potiuk pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/airflow.git from 171ca66142 Fix db clean warnings (#28243) add aace30b50c Fix pre-commit specification for common.sql interface pre-commit (#28278) No new revisions were added by this update. Summary of changes: .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
[GitHub] [airflow] potiuk merged pull request #28278: Fix pre-commit specification for common.sql interface pre-commit
potiuk merged PR #28278: URL: https://github.com/apache/airflow/pull/28278 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] o-nikolas commented on issue #28276: AIP - 51 Make CeleryKubernetesExecutor extends BaseExecutor for a common API.
o-nikolas commented on issue #28276: URL: https://github.com/apache/airflow/issues/28276#issuecomment-1344854419 > @dstandish made me notice that. The consequence is that changes we bring to the `BaseExecutor` are not propagated to the `CeleryKubernetesExecutor`. (`is_local` property for instance, which is pretty counter intuitive and prone to error) Hey Pierre! Yeah, there is some overlap here. I don't love the way multiple executors are stuffed together. I have plans in the works for an AIP to properly support multiple executors, stay tuned for that :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] Taragolis commented on pull request #28187: Add IAM authentication to Amazon Redshift Connection by AWS Connection
Taragolis commented on PR #28187: URL: https://github.com/apache/airflow/pull/28187#issuecomment-1344853490 @shubham22 a chance that you know who might help with review extending integration Redshift Connection and `redshift-connector`? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] Taragolis commented on a diff in pull request #28241: Create Lambda create operator and sensor
Taragolis commented on code in PR #28241: URL: https://github.com/apache/airflow/pull/28241#discussion_r1044888713 ## airflow/providers/amazon/aws/operators/lambda_function.py: ## @@ -20,35 +20,122 @@ import json from typing import TYPE_CHECKING, Sequence +from airflow.compat.functools import cached_property from airflow.models import BaseOperator from airflow.providers.amazon.aws.hooks.lambda_function import LambdaHook if TYPE_CHECKING: from airflow.utils.context import Context -class AwsLambdaInvokeFunctionOperator(BaseOperator): +class LambdaCreateFunctionOperator(BaseOperator): """ -Invokes an AWS Lambda function. -You can invoke a function synchronously (and wait for the response), -or asynchronously. -To invoke a function asynchronously, -set `invocation_type` to `Event`. For more details, -review the boto3 Lambda invoke docs. +Creates an AWS Lambda function. + +More information regarding parameters of this operator can be found here + https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/lambda.html#Lambda.Client.create_function + +.. seealso:: +For more information on how to use this operator, take a look at the guide: +:ref:`howto/operator:AwsLambdaCreateFunctionOperator` Review Comment: The only one small thing that I've found. Reference in documentation has a different name ## docs/apache-airflow-providers-amazon/operators/lambda.rst: ## @@ -33,6 +33,20 @@ Prerequisite Tasks Operators - +.. _howto/operator:AwsLambdaCreateFunctionOperator: + +Create an AWS Lambda function += + +To create an AWS lambda function you can use +:class:`~airflow.providers.amazon.aws.operators.lambda_function.AwsLambdaCreateFunctionOperator`. Review Comment: And vice versa -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] ferruzzi commented on a diff in pull request #28236: Better support for Boto Waiters
ferruzzi commented on code in PR #28236: URL: https://github.com/apache/airflow/pull/28236#discussion_r1044884776 ## airflow/providers/amazon/aws/hooks/eks.py: ## @@ -596,3 +597,6 @@ def fetch_access_token_for_cluster(self, eks_cluster_name: str) -> str: # remove any base64 encoding padding: return "k8s-aws-v1." + base64_url.rstrip("=") + +def get_waiter(self, waiter_name): +return EksBotoWaiter(client=self.conn).waiter(waiter_name) Review Comment: I changed `MANIFEST_TEMPLATE.in.jinja2`. If that's not right, I can fix it. I'll need to rebase before this gets merged anyway. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28278: Fix pre-commit specification for common.sql interface pre-commit
potiuk commented on PR #28278: URL: https://github.com/apache/airflow/pull/28278#issuecomment-1344823280 As noted in #28202 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk opened a new pull request, #28278: Fix pre-commit specification for common.sql interface pre-commit
potiuk opened a new pull request, #28278: URL: https://github.com/apache/airflow/pull/28278 --- **^ Add meaningful description above** Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information. In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed. In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28202: Fix template rendering for Common SQL operators
potiuk commented on PR #28202: URL: https://github.com/apache/airflow/pull/28202#issuecomment-1344821232 > ``` > ^scripts/ci/pre_commit/pre_commit_update_common_sql_api\.py|^airflow/providers/common/sql/.*\.pyi?$ > ``` > to have it run on changes to both `sql.py` and `sql.pyi`, as the static checks action is doing. ^scripts/ci/pre_commit/pre_commit_update_common_sql_api\.py|^airflow/providers/common/sql/.*\.pyi?$ Correct 臘 . The `[i]` is glob not regexp :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] pierrejeambrun commented on pull request #28277: Add is_local to CeleryKubernetesExecutor
pierrejeambrun commented on PR #28277: URL: https://github.com/apache/airflow/pull/28277#issuecomment-1344818460 cc: @o-nikolas -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] pierrejeambrun opened a new pull request, #28277: Add is_local to CeleryKubernetesExecutor
pierrejeambrun opened a new pull request, #28277: URL: https://github.com/apache/airflow/pull/28277 Fix add `is_local` attribute to CeleryKubernetesExecutor. (Doesn't extend BaseExecutor) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] pierrejeambrun commented on issue #28276: AIP - 51 Make CeleryKubernetesExecutor extends BaseExecutor for a common API.
pierrejeambrun commented on issue #28276: URL: https://github.com/apache/airflow/issues/28276#issuecomment-1344814524 @o-nikolas @dstandish made me notice that. The consequence is, changes we bring to the 'BaseExecutor' are not propagated to the `CeleryKubernetesExecutor`. (`is_local` property for instance). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] pierrejeambrun closed pull request #28273: Make CeleryKubernetesExecutor extends BaseExecutor
pierrejeambrun closed pull request #28273: Make CeleryKubernetesExecutor extends BaseExecutor URL: https://github.com/apache/airflow/pull/28273 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] pierrejeambrun opened a new issue, #28276: AIP - 51 Make CeleryKubernetesExecutor extends BaseExecutor for a common API.
pierrejeambrun opened a new issue, #28276: URL: https://github.com/apache/airflow/issues/28276 ### Body Make the `CeleryKubernetesExecutor` extends the `BaseExecutor`. This would allow for a common API and not having to handle this particular executor differently. A solution could be to rework a bit the `BaseExecutor` via property to allow more control for subclasses without breaking backward comp. ### Committer - [X] I acknowledge that I am a maintainer/committer of the Apache Airflow project. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #28275: Weird behaviour of Executor Queues leading to Quarantined test_impersonation_subdag in CI
potiuk commented on issue #28275: URL: https://github.com/apache/airflow/issues/28275#issuecomment-1344812934 cc: @ashb @ephraimbuddy @kaxil @uranusjr Others - I think this one is worth looking at, I looked at it yesterday for a while and I cannot see really why it could happen. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk opened a new issue, #28275: Weird behaviour of Executor Queues leading to Quarantined test_impersonation_subdag in CI
potiuk opened a new issue, #28275: URL: https://github.com/apache/airflow/issues/28275 ### Body I am on a quest now to get rid of the Quarantined tests in CI and I have found a case that indicates that we might have a dangerous race in the core of Airflow executors. This happens rarely and it's next to impossible to reproduce on demand, so I decided to capture it in this issue. Also GitHub Action log will be removed in the future, so I copy the logs and describe the behaviour. The failing case is here: https://github.com/apache/airflow/actions/runs/3652820652/jobs/6171887435 This is the `tests.core.test_impersonation_tests.TestImpersonation testMethod=test_impersonation_subdag` failing - the test is about running a backfill on a subdag with impersonation and it seems that it works fine in most cases, but in some cases it looks like the same task is added twice to the executor Queue and when the task executes, the previously prepared temporary .cfg is already removed by the first execution, when the second execution attempts to start. I looked at the code and various paths, but I could not figure how this can happen. I think it would be great to have more pairs of eyes to look at it. Logs (I added my comments pointing to relevant lines): ``` - Captured stderr call - INFO [airflow.executors.executor_loader] Loaded executor: SequentialExecutor INFO [airflow.executors.sequential_executor.SequentialExecutor] Adding to queue: ['airflow', 'tasks', 'run', 'impersonation_subdag', 'test_subdag_operation', 'backfill__2015-01-01T00:00:00+00:00', '--local', '--pool', 'default_pool', '--subdir', 'DAGS_FOLDER/test_impersonation_subdag.py', '--cfg-path', '/tmp/tmp30wvhstz'] INFO [airflow.executors.sequential_executor.SequentialExecutor] Adding to queue: ['airflow', 'tasks', 'run', 'impersonation_subdag.test_subdag_operation', 'exec_python_fn', 'backfill__2015-01-01T00:00:00+00:00', '--local', '--pool', 'default_pool', '--subdir', 'DAGS_FOLDER/test_impersonation_subdag.py', '--cfg-path', '/tmp/tmpn88_y_ar'] INFO [airflow.executors.sequential_executor.SequentialExecutor] Adding to queue: ['airflow', 'tasks', 'run', 'impersonation_subdag.test_subdag_operation', 'exec_bash_operator', 'backfill__2015-01-01T00:00:00+00:00', '--local', '--pool', 'default_pool', '--subdir', 'DAGS_FOLDER/test_impersonation_subdag.py', '--cfg-path', '/tmp/tmptyfwogyc'] INFO [airflow.executors.sequential_executor.SequentialExecutor] Executing command: ['airflow', 'tasks', 'run', 'impersonation_subdag', 'test_subdag_operation', 'backfill__2015-01-01T00:00:00+00:00', '--local', '--pool', 'default_pool', '--subdir', 'DAGS_FOLDER/test_impersonation_subdag.py', '--cfg-path', '/tmp/tmp30wvhstz'] INFO [airflow.executors.sequential_executor.SequentialExecutor] Executing command: ['airflow', 'tasks', 'run', 'impersonation_subdag.test_subdag_operation', 'exec_python_fn', 'backfill__2015-01-01T00:00:00+00:00', '--local', '--pool', 'default_pool', '--subdir', 'DAGS_FOLDER/test_impersonation_subdag.py', '--cfg-path', '/tmp/tmpn88_y_ar'] INFO [airflow.executors.sequential_executor.SequentialExecutor] Executing command: ['airflow', 'tasks', 'run', 'impersonation_subdag.test_subdag_operation', 'exec_bash_operator', 'backfill__2015-01-01T00:00:00+00:00', '--local', '--pool', 'default_pool', '--subdir', 'DAGS_FOLDER/test_impersonation_subdag.py', '--cfg-path', '/tmp/tmptyfwogyc'] WARNI [airflow.jobs.backfill_job.BackfillJob] Task instance is up for reschedule INFO [airflow.models.dagrun.DagRun] Marking run successful INFO [airflow.models.dagrun.DagRun] DagRun Finished: dag_id=impersonation_subdag.test_subdag_operation, execution_date=2015-01-01T00:00:00+00:00, run_id=backfill__2015-01-01T00:00:00+00:00, run_start_date=2022-12-08 23:35:11.461535+00:00, run_end_date=2022-12-08 23:36:08.586102+00:00, run_duration=57.124567, state=success, external_trigger=False, run_type=backfill, data_interval_start=2015-01-01T00:00:00+00:00, data_interval_end=2015-01-02T00:00:00+00:00, dag_hash=None INFO [airflow.jobs.backfill_job.BackfillJob] [backfill progress] | finished run 1 of 1 | tasks waiting: 1 | succeeded: 2 | running: 0 | failed: 0 | skipped: 0 | deadlocked: 0 | not ready: 0 INFO [airflow.executors.sequential_executor.SequentialExecutor] Adding to queue: ['airflow', 'tasks', 'run', 'impersonation_subdag', 'test_subdag_operation', 'backfill__2015-01-01T00:00:00+00:00', '--local', '--pool', 'default_pool', '--subdir', 'DAGS_FOLDER/test_impersonation_subdag.py', '--cfg-path', '/tmp/tmp4j2_55yd'] ^^ HERE THE TASK IS ADDED TO THE QUEUE (ONCE) INFO [airflow.executors.sequential_executor.SequentialExecutor] Executing command: ['airflow', 'tasks', 'run', 'impersonation_subdag', 'test_subdag_operation',
[GitHub] [airflow] pierrejeambrun commented on pull request #28273: Make CeleryKubernetesExecutor extends BaseExecutor
pierrejeambrun commented on PR #28273: URL: https://github.com/apache/airflow/pull/28273#issuecomment-1344805905 APIs between those 2 are too different, and would require further effort to have it inherit from BaseOperator. (Without having to ignore overrides, skip parent constructor, which could lead to future issue). I think we would need to use properties everywhere so each subclass has better control on how he want to handle it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] vincbeck opened a new pull request, #28274: AWS EC2 system test: wait for instance to exist before launching it
vincbeck opened a new pull request, #28274: URL: https://github.com/apache/airflow/pull/28274 Fix eventual consistency on AWS back-end. Wait for instance to exist before launching it -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] pierrejeambrun commented on a diff in pull request #28273: Make CeleryKubernetesExecutor extends BaseExecutor
pierrejeambrun commented on code in PR #28273: URL: https://github.com/apache/airflow/pull/28273#discussion_r1044822177 ## airflow/executors/celery_kubernetes_executor.py: ## @@ -44,27 +48,27 @@ class CeleryKubernetesExecutor(LoggingMixin): KUBERNETES_QUEUE = conf.get("celery_kubernetes_executor", "kubernetes_queue") def __init__(self, celery_executor: CeleryExecutor, kubernetes_executor: KubernetesExecutor): -super().__init__() +self._set_context(None) self._job_id: int | None = None self.celery_executor = celery_executor self.kubernetes_executor = kubernetes_executor self.kubernetes_executor.kubernetes_queue = self.KUBERNETES_QUEUE @property -def queued_tasks(self) -> dict[TaskInstanceKey, QueuedTaskInstanceType]: +def queued_tasks(self) -> dict[TaskInstanceKey, QueuedTaskInstanceType]: # type: ignore[override] """Return queued tasks from celery and kubernetes executor.""" queued_tasks = self.celery_executor.queued_tasks.copy() queued_tasks.update(self.kubernetes_executor.queued_tasks) return queued_tasks @property -def running(self) -> set[TaskInstanceKey]: +def running(self) -> set[TaskInstanceKey]: # type: ignore[override] """Return running tasks from celery and kubernetes executor.""" return self.celery_executor.running.union(self.kubernetes_executor.running) -@property -def job_id(self) -> int | None: +@property # type: ignore[override] Review Comment: removing one of those 2 ignore raises the supertype incompatibility error. I think it is because of the setter below. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] pierrejeambrun commented on a diff in pull request #28273: Make CeleryKubernetesExecutor extends BaseExecutor
pierrejeambrun commented on code in PR #28273: URL: https://github.com/apache/airflow/pull/28273#discussion_r1044822177 ## airflow/executors/celery_kubernetes_executor.py: ## @@ -44,27 +48,27 @@ class CeleryKubernetesExecutor(LoggingMixin): KUBERNETES_QUEUE = conf.get("celery_kubernetes_executor", "kubernetes_queue") def __init__(self, celery_executor: CeleryExecutor, kubernetes_executor: KubernetesExecutor): -super().__init__() +self._set_context(None) self._job_id: int | None = None self.celery_executor = celery_executor self.kubernetes_executor = kubernetes_executor self.kubernetes_executor.kubernetes_queue = self.KUBERNETES_QUEUE @property -def queued_tasks(self) -> dict[TaskInstanceKey, QueuedTaskInstanceType]: +def queued_tasks(self) -> dict[TaskInstanceKey, QueuedTaskInstanceType]: # type: ignore[override] """Return queued tasks from celery and kubernetes executor.""" queued_tasks = self.celery_executor.queued_tasks.copy() queued_tasks.update(self.kubernetes_executor.queued_tasks) return queued_tasks @property -def running(self) -> set[TaskInstanceKey]: +def running(self) -> set[TaskInstanceKey]: # type: ignore[override] """Return running tasks from celery and kubernetes executor.""" return self.celery_executor.running.union(self.kubernetes_executor.running) -@property -def job_id(self) -> int | None: +@property # type: ignore[override] Review Comment: removing one of those 2 ignore raises an error. I think it is because of the setter below. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] pierrejeambrun opened a new pull request, #28273: Make CeleryKubernetesExecutor extends BaseExecutor
pierrejeambrun opened a new pull request, #28273: URL: https://github.com/apache/airflow/pull/28273 Convert the CeleryKubernetesExecutor to a BaseExecutor. @dstandish Should fix what you mentionned in https://github.com/apache/airflow/commit/42c30724d30e30f82cfc1c517f3c1d4d1e509747#r92580676 We have to `#type: ignore[override]` for now as mypy doesn't allow to override attribute with properties yet (even if getter and setter are both correctly implemented). This is only available in `0.991` https://github.com/python/mypy/commit/40dd719a536589d375ce8ef6cf5f9c6588bbea29 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] Taragolis commented on a diff in pull request #28236: Better support for Boto Waiters
Taragolis commented on code in PR #28236: URL: https://github.com/apache/airflow/pull/28236#discussion_r1044816463 ## airflow/providers/amazon/aws/hooks/eks.py: ## @@ -596,3 +597,6 @@ def fetch_access_token_for_cluster(self, eks_cluster_name: str) -> str: # remove any base64 encoding padding: return "k8s-aws-v1." + base64_url.rstrip("=") + +def get_waiter(self, waiter_name): +return EksBotoWaiter(client=self.conn).waiter(waiter_name) Review Comment: This comment added to generated `MANIFEST.in` into the provider package. I thought it some mistake and it should be `MANIFEST_TEMPLATE.in.jinja2` instead of `MANIFEST_TEMPLATE.py.jinja2`. cc: @potiuk -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on issue #28272: S3KeySensor 'bucket_key' instantiates as a nested list when rendered as a templated_field
boring-cyborg[bot] commented on issue #28272: URL: https://github.com/apache/airflow/issues/28272#issuecomment-1344742998 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] syun64 opened a new issue, #28272: S3KeySensor 'bucket_key' instantiates as a nested list when rendered as a templated_field
syun64 opened a new issue, #28272: URL: https://github.com/apache/airflow/issues/28272 ### Apache Airflow Provider(s) amazon ### Versions of Apache Airflow Providers apache-airflow-providers-amazon==6.2.0 ### Apache Airflow version 2.5.0 ### Operating System Red Hat Enterprise Linux Server 7.6 (Maipo) ### Deployment Virtualenv installation ### Deployment details Simple virtualenv deployment ### What happened bucket_key is a template_field in S3KeySensor, which means that is expected to be rendered as a template field. The supported types for the attribute are both 'str' and 'list'. There is also a [conditional operation in the __init__ function](https://github.com/apache/airflow/blob/main/airflow/providers/amazon/aws/sensors/s3.py#L89) of the class that relies on the type of the input data, that converts the attribute to a list of strings. If a list of str is passed in through Jinja template, **self.bucket_key** is available as a _**doubly-nested list of strings**_, rather than a list of strings. This is because the input value of **bucket_key** can only be a string type that represents the template-string when used as a template_field. These template_fields are then converted to their corresponding values when instantiated as a task_instance. Example log from __init__ function: ` scheduler | DEBUG | type: | val: ["{{ ti.xcom_pull(task_ids='t1') }}"]` Example log from poke function: `poke | DEBUG | type: | val: [["s3://test_bucket/test_key1", "s3://test_bucket/test_key2"]]` This leads to the poke function throwing an [exception](https://github.com/apache/airflow/blob/main/airflow/providers/amazon/aws/hooks/s3.py#L172) as each individual key needs to be a string value to parse the url, but is being passed as a list (since self.bucket_key is a nested list). ### What you think should happen instead Instead of putting the input value of **bucket_key** in a list, we should store the value as-is upon initialization of the class, and just conditionally check the type of the attribute within the poke function. [def \_\_init\_\_](https://github.com/apache/airflow/blob/main/airflow/providers/amazon/aws/sensors/s3.py#L89) `self.bucket_key = bucket_key` (which willstore the input values correctly as a str or a list when the task instance is created and the template fields are rendered) [def poke](https://github.com/apache/airflow/blob/main/airflow/providers/amazon/aws/sensors/s3.py#L127) ``` def poke(self, context: Context): if isinstance(self.bucket_key, str): return self._check_key(key) else: return all(self._check_key(key) for key in self.bucket_key) ``` ### How to reproduce 1. Use a template field as the bucket_key attribute in S3KeySensor 2. Pass a list of strings as the rendered template input value for the bucket_key attribute in the S3KeySensor task. (e.g. as an XCOM or Variable pulled value) Example: ``` with DAG( ... render_template_as_native_obj=True, ) as dag: @task(task_id="get_list_of_str", do_xcom_push=True) def get_list_of_str(): return ["s3://test_bucket/test_key1", "s3://test_bucket/test_key1"] t = get_list_of_str() op = S3KeySensor(task_id="s3_key_sensor", bucket_key="{{ ti.xcom_pull(task_ids='get_list_of_str') }}") t >> op ``` ### Anything else _No response_ ### Are you willing to submit PR? - [X] Yes I am willing to submit a PR! ### Code of Conduct - [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[airflow] branch constraints-main updated: Updating constraints. Build id:
This is an automated email from the ASF dual-hosted git repository. github-bot pushed a commit to branch constraints-main in repository https://gitbox.apache.org/repos/asf/airflow.git The following commit(s) were added to refs/heads/constraints-main by this push: new 1f855f1142 Updating constraints. Build id: 1f855f1142 is described below commit 1f855f1142e17d792dd469ace25ae2607aff2370 Author: Automated GitHub Actions commit AuthorDate: Fri Dec 9 20:13:49 2022 + Updating constraints. Build id: This update in constraints is automatically committed by the CI 'constraints-push' step based on HEAD of '' in '' with commit sha . All tests passed in this build so we determined we can push the updated constraints. See https://github.com/apache/airflow/blob/main/README.md#installing-from-pypi for details. --- constraints-3.10.txt | 2 +- constraints-3.7.txt | 2 +- constraints-3.8.txt | 2 +- constraints-3.9.txt | 2 +- constraints-no-providers-3.10.txt | 2 +- constraints-no-providers-3.7.txt | 2 +- constraints-no-providers-3.8.txt | 2 +- constraints-no-providers-3.9.txt | 2 +- constraints-source-providers-3.10.txt | 4 ++-- constraints-source-providers-3.7.txt | 4 ++-- constraints-source-providers-3.8.txt | 4 ++-- constraints-source-providers-3.9.txt | 4 ++-- 12 files changed, 16 insertions(+), 16 deletions(-) diff --git a/constraints-3.10.txt b/constraints-3.10.txt index ec8144734b..530c7b816b 100644 --- a/constraints-3.10.txt +++ b/constraints-3.10.txt @@ -1,5 +1,5 @@ # -# This constraints file was automatically generated on 2022-12-09T16:27:26Z +# This constraints file was automatically generated on 2022-12-09T20:13:29Z # via "eager-upgrade" mechanism of PIP. For the "main" branch of Airflow. # This variant of constraints install uses the HEAD of the branch version for 'apache-airflow' but installs # the providers from PIP-released packages at the moment of the constraint generation. diff --git a/constraints-3.7.txt b/constraints-3.7.txt index bcb75053d2..5c11f909f3 100644 --- a/constraints-3.7.txt +++ b/constraints-3.7.txt @@ -1,5 +1,5 @@ # -# This constraints file was automatically generated on 2022-12-09T16:28:06Z +# This constraints file was automatically generated on 2022-12-09T20:13:47Z # via "eager-upgrade" mechanism of PIP. For the "main" branch of Airflow. # This variant of constraints install uses the HEAD of the branch version for 'apache-airflow' but installs # the providers from PIP-released packages at the moment of the constraint generation. diff --git a/constraints-3.8.txt b/constraints-3.8.txt index a20be665ae..60e1322f72 100644 --- a/constraints-3.8.txt +++ b/constraints-3.8.txt @@ -1,5 +1,5 @@ # -# This constraints file was automatically generated on 2022-12-09T16:27:57Z +# This constraints file was automatically generated on 2022-12-09T20:13:41Z # via "eager-upgrade" mechanism of PIP. For the "main" branch of Airflow. # This variant of constraints install uses the HEAD of the branch version for 'apache-airflow' but installs # the providers from PIP-released packages at the moment of the constraint generation. diff --git a/constraints-3.9.txt b/constraints-3.9.txt index 713719f155..cc674a4d65 100644 --- a/constraints-3.9.txt +++ b/constraints-3.9.txt @@ -1,5 +1,5 @@ # -# This constraints file was automatically generated on 2022-12-09T16:27:53Z +# This constraints file was automatically generated on 2022-12-09T20:13:38Z # via "eager-upgrade" mechanism of PIP. For the "main" branch of Airflow. # This variant of constraints install uses the HEAD of the branch version for 'apache-airflow' but installs # the providers from PIP-released packages at the moment of the constraint generation. diff --git a/constraints-no-providers-3.10.txt b/constraints-no-providers-3.10.txt index b13c7ce632..a8d92dd49e 100644 --- a/constraints-no-providers-3.10.txt +++ b/constraints-no-providers-3.10.txt @@ -1,5 +1,5 @@ # -# This constraints file was automatically generated on 2022-12-09T16:24:41Z +# This constraints file was automatically generated on 2022-12-09T20:11:43Z # via "eager-upgrade" mechanism of PIP. For the "main" branch of Airflow. # This variant of constraints install just the 'bare' 'apache-airflow' package build from the HEAD of # the branch, without installing any of the providers. diff --git a/constraints-no-providers-3.7.txt b/constraints-no-providers-3.7.txt index d082ab9706..d97024d0eb 100644 --- a/constraints-no-providers-3.7.txt +++ b/constraints-no-providers-3.7.txt @@ -1,5 +1,5 @@ # -# This constraints file was automatically generated on 2022-12-09T16:25:35Z +# This constraints file was automatically generated on 2022-12-09T20:12:11Z # via "eager-upgrade" mechanism of PIP. For the "main" branch of Airflow. # This variant of constraints install just the 'bare' 'apache-airflow' package build from the
[GitHub] [airflow] mhenc opened a new issue, #28271: AIP-44 Migrate Variable to Internal API
mhenc opened a new issue, #28271: URL: https://github.com/apache/airflow/issues/28271 Link: https://github.com/apache/airflow/blob/main/airflow/models/variable.py Methods to migrate: - val - set - delete - update Note that get_variable_from_secrets shouls still be executed locally. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] ferruzzi commented on a diff in pull request #28236: Better support for Boto Waiters
ferruzzi commented on code in PR #28236: URL: https://github.com/apache/airflow/pull/28236#discussion_r1044796643 ## airflow/providers/amazon/aws/hooks/eks.py: ## @@ -596,3 +597,6 @@ def fetch_access_token_for_cluster(self, eks_cluster_name: str) -> str: # remove any base64 encoding padding: return "k8s-aws-v1." + base64_url.rstrip("=") + +def get_waiter(self, waiter_name): +return EksBotoWaiter(client=self.conn).waiter(waiter_name) Review Comment: The file you linked at `dev/provider_packages/MANIFEST_TEMPLATE.in.jinja2` has a message in it saying ``` # NOTE! THIS FILE IS AUTOMATICALLY GENERATED AND WILL BE # OVERWRITTEN WHEN PREPARING PACKAGES. # IF YOU WANT TO MODIFY IT, YOU SHOULD MODIFY THE TEMPLATE # `MANIFEST_TEMPLATE.py.jinja2` IN the `provider_packages` DIRECTORY ``` but there is no file by that name. Do you know if that's an old warning that should be removed/ignored? ``` ferruzzi:~/workplace/airflow$ find . -name MANIFEST_TEMPLATE* ./dev/provider_packages/MANIFEST_TEMPLATE.in.jinja2 ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] mhenc opened a new issue, #28269: AIP-44 Migrate DagFileProcessor.execute_callbacks to Internal API.
mhenc opened a new issue, #28269: URL: https://github.com/apache/airflow/issues/28269 Note that Callback execution must be still done in DagProcessor -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] mhenc opened a new issue, #28267: AIP-44 Provide information to internal_api_call decorator about the running component
mhenc opened a new issue, #28267: URL: https://github.com/apache/airflow/issues/28267 Scheduler/Webserver should never use Internal API, so calling any method decorated with internal_api_call should still execute them locally -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on issue #28266: AIP-44 Implement standalone internal-api component
boring-cyborg[bot] commented on issue #28266: URL: https://github.com/apache/airflow/issues/28266#issuecomment-1344723514 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] ferruzzi commented on a diff in pull request #28236: Better support for Boto Waiters
ferruzzi commented on code in PR #28236: URL: https://github.com/apache/airflow/pull/28236#discussion_r1044796643 ## airflow/providers/amazon/aws/hooks/eks.py: ## @@ -596,3 +597,6 @@ def fetch_access_token_for_cluster(self, eks_cluster_name: str) -> str: # remove any base64 encoding padding: return "k8s-aws-v1." + base64_url.rstrip("=") + +def get_waiter(self, waiter_name): +return EksBotoWaiter(client=self.conn).waiter(waiter_name) Review Comment: The file you linked at `dev/provider_packages/MANIFEST_TEMPLATE.in.jinja2` has a message in it saying ``` # IF YOU WANT TO MODIFY IT, YOU SHOULD MODIFY THE TEMPLATE # `MANIFEST_TEMPLATE.py.jinja2` IN the `provider_packages` DIRECTORY ``` but there is no file by that name. Do you know if that's an old warning that should be removed/ignored? ``` ferruzzi:~/workplace/airflow$ find . -name MANIFEST_TEMPLATE* ./dev/provider_packages/MANIFEST_TEMPLATE.in.jinja2 ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] ferruzzi commented on a diff in pull request #28236: Better support for Boto Waiters
ferruzzi commented on code in PR #28236: URL: https://github.com/apache/airflow/pull/28236#discussion_r1044788681 ## airflow/providers/amazon/aws/hooks/eks.py: ## @@ -596,3 +597,6 @@ def fetch_access_token_for_cluster(self, eks_cluster_name: str) -> str: # remove any base64 encoding padding: return "k8s-aws-v1." + base64_url.rstrip("=") + +def get_waiter(self, waiter_name): +return EksBotoWaiter(client=self.conn).waiter(waiter_name) Review Comment: Oh, thank you. I was unaware of that.I had a chat with Vincent yesterday and he suggested another change so I should have this updated today. Sorry for the delay. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] ferruzzi commented on a diff in pull request #28236: Better support for Boto Waiters
ferruzzi commented on code in PR #28236: URL: https://github.com/apache/airflow/pull/28236#discussion_r1044788085 ## airflow/providers/amazon/aws/waiters/base_waiter.py: ## @@ -0,0 +1,37 @@ +# Licensed to the Apache Software Foundation (ASF) under one Review Comment: Yeah, this would lay the framework for that and adding whatever other custom waiters that folks want to make. The more I look at it, the more I think it'll either simplify or replace a lot of Sensors as well, which wasn't the intended purpose but a very nice side effect. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] joshowen opened a new pull request, #28265: ability to set referenceId in ECSRunTaskOperator
joshowen opened a new pull request, #28265: URL: https://github.com/apache/airflow/pull/28265 --- **^ Add meaningful description above** Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information. In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed. In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[airflow] branch main updated (20a1c4db98 -> 171ca66142)
This is an automated email from the ASF dual-hosted git repository. jedcunningham pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/airflow.git from 20a1c4db98 Add global volume & volumeMounts to the chart (#27781) add 171ca66142 Fix db clean warnings (#28243) No new revisions were added by this update. Summary of changes: airflow/utils/db_cleanup.py| 3 +-- tests/utils/test_db_cleanup.py | 17 + 2 files changed, 18 insertions(+), 2 deletions(-)
[GitHub] [airflow] jedcunningham closed issue #26581: airflow db clean is unable to delete the table rendered_task_instance_fields
jedcunningham closed issue #26581: airflow db clean is unable to delete the table rendered_task_instance_fields URL: https://github.com/apache/airflow/issues/26581 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] jedcunningham merged pull request #28243: Fix db clean warnings
jedcunningham merged PR #28243: URL: https://github.com/apache/airflow/pull/28243 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] jedcunningham commented on pull request #27781: Add global volume & volumeMounts to the chart
jedcunningham commented on PR #27781: URL: https://github.com/apache/airflow/pull/27781#issuecomment-1344666113 Thanks @csp33! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[airflow] branch main updated: Add global volume & volumeMounts to the chart (#27781)
This is an automated email from the ASF dual-hosted git repository. jedcunningham pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/airflow.git The following commit(s) were added to refs/heads/main by this push: new 20a1c4db98 Add global volume & volumeMounts to the chart (#27781) 20a1c4db98 is described below commit 20a1c4db9882a223ae08de1a46e9bdf993698865 Author: Carlos Sánchez Páez AuthorDate: Fri Dec 9 20:07:48 2022 +0100 Add global volume & volumeMounts to the chart (#27781) --- .../dag-processor/dag-processor-deployment.yaml| 6 + chart/templates/flower/flower-deployment.yaml | 6 + chart/templates/jobs/create-user-job.yaml | 6 + chart/templates/jobs/migrate-database-job.yaml | 6 + .../templates/pgbouncer/pgbouncer-deployment.yaml | 6 + .../templates/scheduler/scheduler-deployment.yaml | 12 + .../templates/triggerer/triggerer-deployment.yaml | 9 +++ .../templates/webserver/webserver-deployment.yaml | 9 +++ chart/templates/workers/worker-deployment.yaml | 15 +++ chart/values.schema.json | 18 + chart/values.yaml | 6 + tests/charts/test_create_user_job.py | 16 tests/charts/test_dag_processor.py | 15 +++ tests/charts/test_flower.py| 17 tests/charts/test_migrate_database_job.py | 16 tests/charts/test_pgbouncer.py | 26 +++ tests/charts/test_scheduler.py | 14 ++ tests/charts/test_triggerer.py | 14 ++ tests/charts/test_webserver.py | 30 ++ tests/charts/test_worker.py| 14 ++ 20 files changed, 261 insertions(+) diff --git a/chart/templates/dag-processor/dag-processor-deployment.yaml b/chart/templates/dag-processor/dag-processor-deployment.yaml index fef28af5aa..96abd3ff2b 100644 --- a/chart/templates/dag-processor/dag-processor-deployment.yaml +++ b/chart/templates/dag-processor/dag-processor-deployment.yaml @@ -147,6 +147,9 @@ spec: resources: {{ toYaml .Values.dagProcessor.resources | nindent 12 }} volumeMounts: +{{- if .Values.volumeMounts }} +{{ toYaml .Values.volumeMounts | nindent 12 }} +{{- end }} {{- if .Values.dagProcessor.extraVolumeMounts }} {{ toYaml .Values.dagProcessor.extraVolumeMounts | nindent 12 }} {{- end }} @@ -195,6 +198,9 @@ spec: {{- if and .Values.dags.gitSync.enabled .Values.dags.gitSync.sshKeySecret }} {{- include "git_sync_ssh_key_volume" . | indent 8 }} {{- end }} +{{- if .Values.volumes }} +{{- toYaml .Values.volumes | nindent 8 }} +{{- end }} {{- if .Values.dagProcessor.extraVolumes }} {{- toYaml .Values.dagProcessor.extraVolumes | nindent 8 }} {{- end }} diff --git a/chart/templates/flower/flower-deployment.yaml b/chart/templates/flower/flower-deployment.yaml index ca39b369d4..5c92193502 100644 --- a/chart/templates/flower/flower-deployment.yaml +++ b/chart/templates/flower/flower-deployment.yaml @@ -97,6 +97,9 @@ spec: {{ toYaml .Values.flower.resources | indent 12 }} volumeMounts: {{- include "airflow_config_mount" . | nindent 12 }} +{{- if .Values.volumeMounts }} +{{ toYaml .Values.volumeMounts | nindent 12 }} +{{- end }} {{- if .Values.flower.extraVolumeMounts }} {{ toYaml .Values.flower.extraVolumeMounts | nindent 12 }} {{- end }} @@ -147,6 +150,9 @@ spec: - name: config configMap: name: {{ template "airflow_config" . }} +{{- if .Values.volumes }} +{{- toYaml .Values.volumes | nindent 8 }} +{{- end }} {{- if .Values.flower.extraVolumes }} {{ toYaml .Values.flower.extraVolumes | indent 8 }} {{- end }} diff --git a/chart/templates/jobs/create-user-job.yaml b/chart/templates/jobs/create-user-job.yaml index 784ae05262..a4bb2bde9f 100644 --- a/chart/templates/jobs/create-user-job.yaml +++ b/chart/templates/jobs/create-user-job.yaml @@ -106,6 +106,9 @@ spec: {{ toYaml .Values.createUserJob.resources | indent 12 }} volumeMounts: {{- include "airflow_config_mount" . | nindent 12 }} +{{- if .Values.volumeMounts }} +{{ toYaml .Values.volumeMounts | nindent 12 }} +{{- end }} {{- if .Values.createUserJob.extraVolumeMounts }} {{ toYaml .Values.createUserJob.extraVolumeMounts | nindent 12 }} {{- end }} @@ -116,6 +119,9 @@ spec: - name: config configMap: name: {{ template "airflow_config" . }} +{{- if .Values.volumes }} +{{- toYaml .Values.volumes | nindent 8 }} +{{- end }} {{- if .Values.createUserJob.extraVolumes }} {{ toYaml
[GitHub] [airflow] jedcunningham closed issue #27687: Allow global volume & volumeMounts in Helm chart
jedcunningham closed issue #27687: Allow global volume & volumeMounts in Helm chart URL: https://github.com/apache/airflow/issues/27687 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] jedcunningham merged pull request #27781: Add global volume & volumeMounts to the chart
jedcunningham merged PR #27781: URL: https://github.com/apache/airflow/pull/27781 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] vincbeck commented on a diff in pull request #28024: Add AWS SageMaker operator to register a model's version
vincbeck commented on code in PR #28024: URL: https://github.com/apache/airflow/pull/28024#discussion_r1044731751 ## airflow/providers/amazon/aws/operators/sagemaker.py: ## @@ -750,3 +751,99 @@ def execute(self, context: Context) -> Any: sagemaker_hook = SageMakerHook(aws_conn_id=self.aws_conn_id) sagemaker_hook.delete_model(model_name=self.config["ModelName"]) self.log.info("Model %s deleted successfully.", self.config["ModelName"]) + + +class ApprovalStatus(Enum): +"""Approval statuses for a Sagemaker Model Package.""" + +APPROVED = "Approved" +REJECTED = "Rejected" +PENDING_MANUAL_APPROVAL = "PendingManualApproval" + + +class SageMakerRegisterModelVersionOperator(SageMakerBaseOperator): +""" +Registers an Amazon SageMaker model by creating a model version that specifies the model group to which it +belongs. Will create the model group if it does not exist already. + +.. seealso:: +For more information on how to use this operator, take a look at the guide: +:ref:`howto/operator:SageMakerRegisterModelVersionOperator` + +:param image_uri: The Amazon EC2 Container Registry (Amazon ECR) path where inference code is stored. +:param model_url: The Amazon S3 path where the model artifacts (the trained weights of the model), which +result from model training, are stored. This path must point to a single gzip compressed tar archive +(.tar.gz suffix). +:param package_group_name: The name of the model package group that the model is going to be registered +to. Will be created if it doesn't already exist. +:param package_group_desc: Description of the model package group, if it was to be created (optional). +:param package_desc: Description of the model package (optional). +:param model_approval: Approval status of the model package. Defaults to PendingManualApproval +:param aws_conn_id: The AWS connection ID to use. +:param config: Can contain extra parameters for the boto call to create_model_package, and/or overrides +for any parameter defined above. For a complete list of available parameters, see + https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/sagemaker.html#SageMaker.Client.create_model_package Review Comment: I guess it is question of opinion but my personal opinion is we should list all parameters regardless of if they are being used in that class or in an underlying class ## airflow/providers/amazon/aws/operators/sagemaker.py: ## @@ -750,3 +751,99 @@ def execute(self, context: Context) -> Any: sagemaker_hook = SageMakerHook(aws_conn_id=self.aws_conn_id) sagemaker_hook.delete_model(model_name=self.config["ModelName"]) self.log.info("Model %s deleted successfully.", self.config["ModelName"]) + + +class ApprovalStatus(Enum): +"""Approval statuses for a Sagemaker Model Package.""" + +APPROVED = "Approved" +REJECTED = "Rejected" +PENDING_MANUAL_APPROVAL = "PendingManualApproval" Review Comment: Agree ## tests/system/providers/amazon/aws/example_sagemaker.py: ## @@ -539,12 +564,14 @@ def delete_logs(env_id): train_model, await_training, create_model, +register_model, tune_model, await_tuning, test_model, await_transform, # TEST TEARDOWN delete_ecr_repository(test_setup["ecr_repository_name"]), +delete_model_group(test_setup["model_package_group_name"], register_model.output), Review Comment: In that case, this task will fail as well. This happens a lot in AWS system tests with teardown tasks. Another (and more simple) example is, a S3 bucket is created at the beginning of a system test, there is a teardown task to delete this bucket at the end of the system test. What happens if the bucket does not even get created because an exception is thrown before or during the creation? The deletion fails as well. So far we are okay with this behavior ## tests/system/providers/amazon/aws/example_sagemaker.py: ## @@ -421,6 +429,14 @@ def delete_logs(env_id): purge_logs(generated_logs) +@task(trigger_rule=TriggerRule.ALL_DONE) +def delete_model_group(group_name, model_version_arn): +sgmk_client = boto3.client("sagemaker") +# need to destroy model registered in group first +sgmk_client.delete_model_package(ModelPackageName=model_version_arn) +sgmk_client.delete_model_package_group(ModelPackageGroupName=group_name) + Review Comment: No plan yet. We might want to create this operator later but again, there is no plan as of today -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this
[GitHub] [airflow] Taragolis commented on issue #27078: Connection time out when doing sts_assume_role with AwsBaseHook
Taragolis commented on issue #27078: URL: https://github.com/apache/airflow/issues/27078#issuecomment-134460 Oh... right. Forget that STS didn't use regional endpoint by default in `boto3`. And it should be enabled by set environment variable `AWS_STS_REGIONAL_ENDPOINTS` to `regional` all other clients use regional endpoints by default (except some S3 regions) ```python import os import boto3 os.environ["AWS_DEFAULT_REGION"] = "eu-west-1" session = boto3.session.Session() emr_client = session.client("emr") print(f"Client info: {emr_client.meta.config.user_agent}") print(f"STS Endpoint URL: {emr_client.meta.endpoint_url}") sts_client = session.client("sts") print(f"STS Endpoint URL: {sts_client.meta.endpoint_url}") os.environ["AWS_STS_REGIONAL_ENDPOINTS"] = "regional" sts_client_regional = session.client("sts") print(f"STS Endpoint URL: {sts_client_regional.meta.endpoint_url}") ``` I have an idea to enable set endpoint url by dictionary (on service level) in connection rather than define it for all services. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on pull request #28264: Fix helm chart NOTES.txt to show correct URL
boring-cyborg[bot] commented on PR #28264: URL: https://github.com/apache/airflow/pull/28264#issuecomment-1344628731 Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst) Here are some useful points: - Pay attention to the quality of your code (flake8, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that. - In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it. - Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/BREEZE.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations. - Be patient and persistent. It might take some time to get a review or get the final approval from Committers. - Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack. - Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#coding-style-and-best-practices). Apache Airflow is a community-driven project and together we are making it better . In case of doubts contact the developers at: Mailing List: d...@airflow.apache.org Slack: https://s.apache.org/airflow-slack -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] okue opened a new pull request, #28264: Fix helm chart NOTES.txt to show correct URL
okue opened a new pull request, #28264: URL: https://github.com/apache/airflow/pull/28264 ## What happened `breeze k8s deploy-airflow --upgrade` results is https://user-images.githubusercontent.com/13622816/206767539-2e1aae5d-8b7f-499c-b5cc-03663b0943b7.png;> with `values.yaml` shown below. ```yaml ingress: web: enabled: true annotations: {} path: "/" pathType: "ImplementationSpecific" hosts: - name: "localhost" tls: enabled: false secretName: "" ``` Airflow Webserver URL is incorrect. ## What you think should happen instead ``` Airflow Webserver: http: ``` should be ``` Airflow Webserver: http://localhost/{path}/ ``` ## Modifications in this PR - Fix NOTES.txt --- **^ Add meaningful description above** Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information. In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed. In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] Taragolis commented on pull request #28248: Hopefully stabilize quarantined tests for celery hanging
Taragolis commented on PR #28248: URL: https://github.com/apache/airflow/pull/28248#issuecomment-1344617175 I went one commit behind https://github.com/apache/airflow/commit/f75dd7ae6e755dad328ba6f3fd462ade194dab25 and adopt test to old implementation as result only one thing I could achieve - my laptop stated work as a heater but test still not 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] jon-evergreen commented on pull request #28202: Fix template rendering for Common SQL operators
jon-evergreen commented on PR #28202: URL: https://github.com/apache/airflow/pull/28202#issuecomment-1344578092 Also, in case it wasn't clear, I have now checked in the updated `sql.pyi` file -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] XD-DENG commented on pull request #28047: KubernetesExecutor multi_namespace_mode can use namespace list
XD-DENG commented on PR #28047: URL: https://github.com/apache/airflow/pull/28047#issuecomment-1344546028 Thanks a lot @potiuk for helping check and sharing the Breeze/CI tips! I will do another check later today. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] snjypl closed pull request #28161: WIP AIP-51 - Executor Coupling in Logging
snjypl closed pull request #28161: WIP AIP-51 - Executor Coupling in Logging URL: https://github.com/apache/airflow/pull/28161 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28248: Hopefully stabilize quarantined tests for celery hanging
potiuk commented on PR #28248: URL: https://github.com/apache/airflow/pull/28248#issuecomment-1344538663 Yeah. I call them Heisentests - the more you look at them, the leß likely they fail. But the moment you turn your head, they start failing. Been there, done that -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] bharanidharan14 opened a new pull request, #28262: [WIP]: Hook for managing directories and files in Azure Data Lake Storage Gen2
bharanidharan14 opened a new pull request, #28262: URL: https://github.com/apache/airflow/pull/28262 - created hook for supporting ADLS gen2 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[airflow] branch constraints-main updated: Updating constraints. Build id:
This is an automated email from the ASF dual-hosted git repository. github-bot pushed a commit to branch constraints-main in repository https://gitbox.apache.org/repos/asf/airflow.git The following commit(s) were added to refs/heads/constraints-main by this push: new 5a00c28833 Updating constraints. Build id: 5a00c28833 is described below commit 5a00c28833125540b01224376cf5b405cf816ba4 Author: Automated GitHub Actions commit AuthorDate: Fri Dec 9 16:28:09 2022 + Updating constraints. Build id: This update in constraints is automatically committed by the CI 'constraints-push' step based on HEAD of '' in '' with commit sha . All tests passed in this build so we determined we can push the updated constraints. See https://github.com/apache/airflow/blob/main/README.md#installing-from-pypi for details. --- constraints-3.10.txt | 4 ++-- constraints-3.7.txt | 4 ++-- constraints-3.8.txt | 4 ++-- constraints-3.9.txt | 4 ++-- constraints-no-providers-3.10.txt | 2 +- constraints-no-providers-3.7.txt | 2 +- constraints-no-providers-3.8.txt | 2 +- constraints-no-providers-3.9.txt | 2 +- constraints-source-providers-3.10.txt | 2 +- constraints-source-providers-3.7.txt | 2 +- constraints-source-providers-3.8.txt | 2 +- constraints-source-providers-3.9.txt | 2 +- 12 files changed, 16 insertions(+), 16 deletions(-) diff --git a/constraints-3.10.txt b/constraints-3.10.txt index c9f89aeb2e..ec8144734b 100644 --- a/constraints-3.10.txt +++ b/constraints-3.10.txt @@ -1,5 +1,5 @@ # -# This constraints file was automatically generated on 2022-12-08T22:46:35Z +# This constraints file was automatically generated on 2022-12-09T16:27:26Z # via "eager-upgrade" mechanism of PIP. For the "main" branch of Airflow. # This variant of constraints install uses the HEAD of the branch version for 'apache-airflow' but installs # the providers from PIP-released packages at the moment of the constraint generation. @@ -170,7 +170,7 @@ backoff==1.10.0 bcrypt==4.0.1 beautifulsoup4==4.11.1 billiard==3.6.4.0 -black==22.10.0 +black==22.12.0 bleach==5.0.1 blinker==1.5 boto3==1.26.26 diff --git a/constraints-3.7.txt b/constraints-3.7.txt index 042903c0bb..bcb75053d2 100644 --- a/constraints-3.7.txt +++ b/constraints-3.7.txt @@ -1,5 +1,5 @@ # -# This constraints file was automatically generated on 2022-12-08T22:46:51Z +# This constraints file was automatically generated on 2022-12-09T16:28:06Z # via "eager-upgrade" mechanism of PIP. For the "main" branch of Airflow. # This variant of constraints install uses the HEAD of the branch version for 'apache-airflow' but installs # the providers from PIP-released packages at the moment of the constraint generation. @@ -170,7 +170,7 @@ backports.zoneinfo==0.2.1 bcrypt==4.0.1 beautifulsoup4==4.11.1 billiard==3.6.4.0 -black==22.10.0 +black==22.12.0 bleach==5.0.1 blinker==1.5 boto3==1.26.26 diff --git a/constraints-3.8.txt b/constraints-3.8.txt index d09fb9e596..a20be665ae 100644 --- a/constraints-3.8.txt +++ b/constraints-3.8.txt @@ -1,5 +1,5 @@ # -# This constraints file was automatically generated on 2022-12-08T22:46:46Z +# This constraints file was automatically generated on 2022-12-09T16:27:57Z # via "eager-upgrade" mechanism of PIP. For the "main" branch of Airflow. # This variant of constraints install uses the HEAD of the branch version for 'apache-airflow' but installs # the providers from PIP-released packages at the moment of the constraint generation. @@ -171,7 +171,7 @@ backports.zoneinfo==0.2.1 bcrypt==4.0.1 beautifulsoup4==4.11.1 billiard==3.6.4.0 -black==22.10.0 +black==22.12.0 bleach==5.0.1 blinker==1.5 boto3==1.26.26 diff --git a/constraints-3.9.txt b/constraints-3.9.txt index 0319131879..713719f155 100644 --- a/constraints-3.9.txt +++ b/constraints-3.9.txt @@ -1,5 +1,5 @@ # -# This constraints file was automatically generated on 2022-12-08T22:46:44Z +# This constraints file was automatically generated on 2022-12-09T16:27:53Z # via "eager-upgrade" mechanism of PIP. For the "main" branch of Airflow. # This variant of constraints install uses the HEAD of the branch version for 'apache-airflow' but installs # the providers from PIP-released packages at the moment of the constraint generation. @@ -170,7 +170,7 @@ backoff==1.10.0 bcrypt==4.0.1 beautifulsoup4==4.11.1 billiard==3.6.4.0 -black==22.10.0 +black==22.12.0 bleach==5.0.1 blinker==1.5 boto3==1.26.26 diff --git a/constraints-no-providers-3.10.txt b/constraints-no-providers-3.10.txt index 1d8889493d..b13c7ce632 100644 --- a/constraints-no-providers-3.10.txt +++ b/constraints-no-providers-3.10.txt @@ -1,5 +1,5 @@ # -# This constraints file was automatically generated on 2022-12-08T22:44:48Z +# This constraints file was automatically generated on 2022-12-09T16:24:41Z # via "eager-upgrade" mechanism of PIP. For the "main" branch of
[GitHub] [airflow] rodrigocollavo commented on issue #19251: Add a way to skip the secret_backend
rodrigocollavo commented on issue #19251: URL: https://github.com/apache/airflow/issues/19251#issuecomment-1344491172 I had to extend the functionality to make it works with airflow connections as well, adding the following function to the code mentioned above: ``` def get_conn_value(self, key: str) -> Optional[str]: if self.connections_prefix is None: return None if self.secret_lookup_prefix is not None: if not key.startswith(self.secret_lookup_prefix): return None secret = self._get_secret(self.connections_prefix, key) return secret ``` @maxexcloo probably you would like to add it to your package. I've tried yours first, but then I had to create a custom one to support connections in the secret manager. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] Taragolis commented on pull request #28248: Hopefully stabilize quarantined tests for celery hanging
Taragolis commented on PR #28248: URL: https://github.com/apache/airflow/pull/28248#issuecomment-1344469240 Unfortunetly I've unable to reproduce bug in both cases locally. Nor in loop nor by `flacky` might be as you mention it not applicable now. I would try locally revert changes which solve initial issue and try it again. I have some flashback: about 3 years ago we have issue with same symptoms when process running in `mp` just randomly hangs and this only happen in production but never reproduce locally. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] tirkarthi commented on pull request #28256: Include full path to Python files under zip path while clearing import errors.
tirkarthi commented on PR #28256: URL: https://github.com/apache/airflow/pull/28256#issuecomment-1344459058 Thanks @potiuk, I updated my branch with main before creating the PR but it seems some other PRs were merged after my PR. I will rebase in future in case of test failures. Thanks again. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28256: Include full path to Python files under zip path while clearing import errors.
potiuk commented on PR #28256: URL: https://github.com/apache/airflow/pull/28256#issuecomment-139424 > The test failure seems to be unrelated to the pull request. You can always rebase if unsure. I just did (but feel free next time to just do it) - if you see that your branch is not based on the latest main, rebasing usually the best idea, and you can always do it without waiting for anyone else. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #27952: can not use output of task decorator as input for external_task_ids of ExternalTaskSensor
potiuk commented on issue #27952: URL: https://github.com/apache/airflow/issues/27952#issuecomment-135058 Ah ok. Thanks. yep. Not fixed it seems. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[airflow] branch main updated (5dfdeeeac0 -> d8a06581d4)
This is an automated email from the ASF dual-hosted git repository. potiuk pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/airflow.git from 5dfdeeeac0 Dynamically forward ports from trino integration service to host (#28257) add d8a06581d4 Added --integration flag to "standard" set of flags for testing command (#28261) No new revisions were added by this update. Summary of changes: .../commands/testing_commands_config.py| 1 + images/breeze/output-commands.svg | 88 - images/breeze/output_setup.svg | 26 ++--- .../output_setup_regenerate-command-images.svg | 54 +- images/breeze/output_shell.svg | 100 +-- images/breeze/output_start-airflow.svg | 110 ++--- images/breeze/output_static-checks.svg | 4 +- images/breeze/output_testing.svg | 24 ++--- images/breeze/output_testing_integration-tests.svg | 58 +-- images/breeze/output_testing_tests.svg | 90 - 10 files changed, 278 insertions(+), 277 deletions(-)
[GitHub] [airflow] potiuk merged pull request #28261: Added --integration flag to "standard" set of flags for testing command
potiuk merged PR #28261: URL: https://github.com/apache/airflow/pull/28261 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28259: Speed up most Users/Role CLI commands
potiuk commented on PR #28259: URL: https://github.com/apache/airflow/pull/28259#issuecomment-1344434642 Some tests will need fixes - of course :). Will do it tomorrow. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] MaiHoangViet1809 commented on issue #27952: can not use output of task decorator as input for external_task_ids of ExternalTaskSensor
MaiHoangViet1809 commented on issue #27952: URL: https://github.com/apache/airflow/issues/27952#issuecomment-1344420984 the log from v2.5.0 is below: ``` Broken DAG: [//airflow/dags/reproduce_error_for_27952.py] Traceback (most recent call last): File "//env3.10.5/lib/python3.10/site-packages/airflow/models/baseoperator.py", line 411, in apply_defaults result = func(self, **kwargs, default_args=default_args) File "//env3.10.5/lib/python3.10/site-packages/airflow/sensors/external_task.py", line 165, in __init__ if external_task_ids and len(external_task_ids) > len(set(external_task_ids)): TypeError: object of type 'PlainXComArg' has no len() ``` note: masked path with -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #27952: can not use output of task decorator as input for external_task_ids of ExternalTaskSensor
potiuk commented on issue #27952: URL: https://github.com/apache/airflow/issues/27952#issuecomment-1344395945 Can you post the logs from v2.5.0 showing the error (I am asking this for the third time, so please don't make me ask it again - just reproduce and post it). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28248: Hopefully stabilize quarantined tests for celery hanging
potiuk commented on PR #28248: URL: https://github.com/apache/airflow/pull/28248#issuecomment-1344392785 Feel free to try :). Though I think it could be a different behaviour (the test is not mine - just read the commit message it came with: https://github.com/apache/airflow/commit/f75dd7ae6e755dad328ba6f3fd462ade194dab25 ) I belive the reason was that it could be triggered by a quick succession of those celery calls one-by-one and if we wrap it with flaky, it might simply never trigger. @yuqian90 - maybe you still remember this commit and the case and can shed some light on it In this case the whole idea of this test is not the best - I understand it was a way to make more certaintly that we do not trigger the case, but in essence this test is "flaky by design" when something happens so we might as well ignore it in the future (which we did by adding it to quarantined tests) - without actually knowing if the problem is still there or not. I believe though - that the problem is generally solved by the change, and maybe we do not even need to keep the test and can simply remove it (because regression is a) not likely b) we might not even notice the regression because of the flakey nature of the test). So it might be more distraction than help. @Taragolis : Yes, Using flaky might be a good idea in general. The problem with quarantined tests though is not only them being flaky - but also them having side-effects on other tests. Most of the tests in the "quarantined" group that we have had this nasty property, that they not only failed from time to time, but their sheer presence in the "main" group of tests caused the other tests to be also affected (that's why we invented the "quarantine" idea) to keep them isolated from the main pytest run. (and yest it was at the beginnig of COVID so the name is inspired by real-life events :D). So "flaky" library is a bit different concept. Bu yes - I would love to get rid of the quarantined tests (while leaving the possibility of quarantining some tests if we find them breaking things again). I guess we could attempt (one-by-one ideally) to fix those tests and possibly move the quarantined tests to be "flaky" ones and see if that works. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] MaiHoangViet1809 commented on issue #27952: can not use output of task decorator as input for external_task_ids of ExternalTaskSensor
MaiHoangViet1809 commented on issue #27952: URL: https://github.com/apache/airflow/issues/27952#issuecomment-1344387121 > But did you actually reproduce it ? @MaiHoangViet1809 ? > > I belive the problem has been solved in the way that len(external_task_ids) will not raise the exception in the first place (this was the fix in #27251). Please double-check if you can reproduce it there and post logs in case it is not. oh, #27251 resolved another issues I reported before this, #27209. this one is still error in v2.5.0 after I upgraded airflow from 2.4.3 to 2.5.0 error log is the same with v2.4.3. I did try to move that 2 lines in "airflow/airflow/sensors/external_task.py" to the poke method of class and it work, but I am not sure which other line need to move so I fired this issue. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #27952: can not use output of task decorator as input for external_task_ids of ExternalTaskSensor
potiuk commented on issue #27952: URL: https://github.com/apache/airflow/issues/27952#issuecomment-1344376497 But did you actually reproduce it ? @MaiHoangViet1809 ? I belive the problem has been solved in the way that len(external_task_ids) will not raise the exception in the first place (this was the fix in #27251). Please double-check if you can reproduce it there and post logs in case it is not. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on a diff in pull request #28259: Speed up most Users/Role CLI commands
potiuk commented on code in PR #28259: URL: https://github.com/apache/airflow/pull/28259#discussion_r1044495547 ## airflow/cli/commands/user_command.py: ## @@ -49,42 +48,47 @@ class UserSchema(Schema): @suppress_logs_and_warning def users_list(args): """Lists users at the command line.""" -appbuilder = cached_app().appbuilder -users = appbuilder.sm.get_all_users() -fields = ["id", "username", "email", "first_name", "last_name", "roles"] +from airflow.utils.cli_app_builder import get_application_builder -AirflowConsole().print_as( -data=users, output=args.output, mapper=lambda x: {f: x.__getattribute__(f) for f in fields} -) +with get_application_builder() as appbuilder: +users = appbuilder.sm.get_all_users() +fields = ["id", "username", "email", "first_name", "last_name", "roles"] + +AirflowConsole().print_as( +data=users, output=args.output, mapper=lambda x: {f: x.__getattribute__(f) for f in fields} +) @cli_utils.action_cli(check_db=True) def users_create(args): """Creates new user in the DB.""" -appbuilder = cached_app().appbuilder -role = appbuilder.sm.find_role(args.role) -if not role: -valid_roles = appbuilder.sm.get_all_roles() -raise SystemExit(f"{args.role} is not a valid role. Valid roles are: {valid_roles}") - -if args.use_random_password: -password = "".join(random.choice(string.printable) for _ in range(16)) -elif args.password: -password = args.password -else: -password = getpass.getpass("Password:") -password_confirmation = getpass.getpass("Repeat for confirmation:") -if password != password_confirmation: -raise SystemExit("Passwords did not match") - -if appbuilder.sm.find_user(args.username): -print(f"{args.username} already exist in the db") -return -user = appbuilder.sm.add_user(args.username, args.firstname, args.lastname, args.email, role, password) -if user: -print(f'User "{args.username}" created with role "{args.role}"') -else: -raise SystemExit("Failed to create user") +from airflow.utils.cli_app_builder import get_application_builder + +with get_application_builder() as appbuilder: Review Comment: I do not want to make any more changes. Pareto's rule again - 20% of cost getting 80% gain. Sooner or later we WILL get rid of Fab in t's current form so those commands should be refactored then. For now - I think we have to live what we have and make our dependencies smaller and smaller to the point that such a replacement will be easy. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28047: KubernetesExecutor multi_namespace_mode can use namespace list
potiuk commented on PR #28047: URL: https://github.com/apache/airflow/pull/28047#issuecomment-1344354789 My guess is - this is problem with cleanup. You have a LOT of parameterized tests and they are not cleaned up after each test. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28047: KubernetesExecutor multi_namespace_mode can use namespace list
potiuk commented on PR #28047: URL: https://github.com/apache/airflow/pull/28047#issuecomment-1344350801 Yep @XD-DENG - precisely as I expected: At some point in time those tests start eating memory at the right of > 1GB / 10 seconds. Just before the crash we have just 800 MB left, Something spins out of control and causes that - this only happens on your change, not all the other PRs or main - so it **MUST** be something here that triggers it. Now we just need to find out what it might be: ![image](https://user-images.githubusercontent.com/595491/206719600-6f5bf3a7-0d4d-46c7-983b-a6b05aade407.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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk opened a new pull request, #28261: Added --integration flag to "standard" set of flags for testing command
potiuk opened a new pull request, #28261: URL: https://github.com/apache/airflow/pull/28261 The --integration tag should be standard flag rather than left for the common options. This change moves the flag to the right group. Images were regenerated because Rich does not know that the commands changed just when the option was moved to another group (this is a rich-click configuration and we are generating hash of commands from rich's command definition. As result of it, some of the breeze's svg files are changed. There are still subtle differences (mainly about font specification) on Linux and Maci and possibly it depends on what fonts are installed on your system - so when you regenerate images, font definition changes. This should be no issue in general as those images have the same hash and for all practical purposes, they are unchanged. --- **^ Add meaningful description above** Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information. In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed. In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28047: KubernetesExecutor multi_namespace_mode can use namespace list
potiuk commented on PR #28047: URL: https://github.com/apache/airflow/pull/28047#issuecomment-1344336019 More options available (such as ``-debug-resources` in `--help` output: ![image](https://user-images.githubusercontent.com/595491/206717640-58f49ae6-dc5b-4965-ba81-d5bcf2ac66cf.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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28047: KubernetesExecutor multi_namespace_mode can use namespace list
potiuk commented on PR #28047: URL: https://github.com/apache/airflow/pull/28047#issuecomment-1344334903 And even if you do not have as big of a machine you can control number of parallel tests by ``--parallelism`` flag to stress your system even more. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #28047: KubernetesExecutor multi_namespace_mode can use namespace list
potiuk commented on PR #28047: URL: https://github.com/apache/airflow/pull/28047#issuecomment-1344333515 BTW. You can also locally do this: ``` breeze testing tests --run-in-parallell ``` This will do EXACTLY what is being done in the CI - it will run all the tests in parallel in the same way as in CI - if you have big enough local machine (> 32 GB for docker) 8 CPUS - it will run with the same number of parallel tests as the tests we run in CI, so if there is any side effect of those tests running in parallel (there should not be because each of them uses a separate `docker-compose` started - you will be able to see it there yourself (I.e. how memory usage grows when you run the tests in parallel). This should enable you to fully reproduce locally what you see in 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. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] malthe commented on a diff in pull request #28259: Speed up most Users/Role CLI commands
malthe commented on code in PR #28259: URL: https://github.com/apache/airflow/pull/28259#discussion_r1044469613 ## airflow/cli/commands/user_command.py: ## @@ -49,42 +48,47 @@ class UserSchema(Schema): @suppress_logs_and_warning def users_list(args): """Lists users at the command line.""" -appbuilder = cached_app().appbuilder -users = appbuilder.sm.get_all_users() -fields = ["id", "username", "email", "first_name", "last_name", "roles"] +from airflow.utils.cli_app_builder import get_application_builder -AirflowConsole().print_as( -data=users, output=args.output, mapper=lambda x: {f: x.__getattribute__(f) for f in fields} -) +with get_application_builder() as appbuilder: +users = appbuilder.sm.get_all_users() +fields = ["id", "username", "email", "first_name", "last_name", "roles"] + +AirflowConsole().print_as( +data=users, output=args.output, mapper=lambda x: {f: x.__getattribute__(f) for f in fields} +) @cli_utils.action_cli(check_db=True) def users_create(args): """Creates new user in the DB.""" -appbuilder = cached_app().appbuilder -role = appbuilder.sm.find_role(args.role) -if not role: -valid_roles = appbuilder.sm.get_all_roles() -raise SystemExit(f"{args.role} is not a valid role. Valid roles are: {valid_roles}") - -if args.use_random_password: -password = "".join(random.choice(string.printable) for _ in range(16)) -elif args.password: -password = args.password -else: -password = getpass.getpass("Password:") -password_confirmation = getpass.getpass("Repeat for confirmation:") -if password != password_confirmation: -raise SystemExit("Passwords did not match") - -if appbuilder.sm.find_user(args.username): -print(f"{args.username} already exist in the db") -return -user = appbuilder.sm.add_user(args.username, args.firstname, args.lastname, args.email, role, password) -if user: -print(f'User "{args.username}" created with role "{args.role}"') -else: -raise SystemExit("Failed to create user") +from airflow.utils.cli_app_builder import get_application_builder + +with get_application_builder() as appbuilder: Review Comment: As a general code comment, splitting this sort of logic out into a separate function would alleviate having these big diffs where really it's a context manager that's changing. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] tirkarthi commented on pull request #28256: Include full path to Python files under zip path while clearing import errors.
tirkarthi commented on PR #28256: URL: https://github.com/apache/airflow/pull/28256#issuecomment-1344327963 The test failure seems to be unrelated to the pull request. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org