[GitHub] [airflow] shahar1 commented on pull request #27797: Add tests to PythonOperator

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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)

2022-12-09 Thread xddeng
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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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)

2022-12-09 Thread dstandish
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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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:

2022-12-09 Thread github-bot
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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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.

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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.

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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)

2022-12-09 Thread potiuk
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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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)"

2022-12-09 Thread GitBox


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)"

2022-12-09 Thread GitBox


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)

2022-12-09 Thread potiuk
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

2022-12-09 Thread GitBox


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.

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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.

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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.

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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:

2022-12-09 Thread github-bot
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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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.

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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)

2022-12-09 Thread jedcunningham
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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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)

2022-12-09 Thread jedcunningham
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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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:

2022-12-09 Thread github-bot
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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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.

2022-12-09 Thread GitBox


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.

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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)

2022-12-09 Thread potiuk
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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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

2022-12-09 Thread GitBox


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.

2022-12-09 Thread GitBox


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



  1   2   >