[GitHub] [airflow] ramdharam commented on issue #9542: on_failure_callback function for external task sensor
ramdharam commented on issue #9542: URL: https://github.com/apache/airflow/issues/9542#issuecomment-653720188 I can pick this up if this seems a valid use case. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] tirkarthi opened a new pull request #9649: Import ABC from collections.abc
tirkarthi opened a new pull request #9649: URL: https://github.com/apache/airflow/pull/9649 `collections.abc` should be used while using ABC. `collections` is used which is incorrect. One of the other dependencies like logging import `collections.abc` and thus shadows this error. Relevant line : https://github.com/apache/airflow/blob/5e4b801b32eeda79b59ff3cc8a3a503f57f5a509/airflow/utils/email.py#L132 --- Make sure to mark the boxes below before creating PR: [x] - [x] Description above provides context of the change - [ ] Unit tests coverage for changes (not needed for documentation changes) - [ ] Target Github ISSUE in description if exists - [ ] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)" - [ ] Relevant documentation is updated including usage instructions. - [ ] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example). --- In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed. In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md). Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] j-y-matsubara commented on pull request #9531: Support .airflowignore for plugins
j-y-matsubara commented on pull request #9531: URL: https://github.com/apache/airflow/pull/9531#issuecomment-653715178 > @j-y-matsubara would you mind a rebase? The k8s tests should be fixed now Sure. All tests were passed! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[airflow] tag nightly-master updated (63a8c79 -> 5e4b801)
This is an automated email from the ASF dual-hosted git repository.
github-bot pushed a change to tag nightly-master
in repository https://gitbox.apache.org/repos/asf/airflow.git.
*** WARNING: tag nightly-master was modified! ***
from 63a8c79 (commit)
to 5e4b801 (commit)
from 63a8c79 Replace old SubDag zoom screenshot with new (#9621)
add cd3d9d9 Fix using .json template extension in GMP operators (#9566)
add 5cf2585 Fix docstrings in exceptions.py (#9622)
add e50e946 Task logging handlers can provide custom log links (#9354)
add ce9bad4 Improve queries number SchedulerJob._process_executor_events
(#9488)
add ee20086 Move S3TaskHandler to the AWS provider package (#9602)
add 611d449 Use supports_read instead of is_supported in log endpoint
(#9628)
add 37ca8ad Updated link to official documentation (#9629)
add 72d5a58 Fixing typo in chart/README.me (#9632)
add fddc572 Customizable page size limit in API (#9431)
add a99aaeb Allow setting Hashicorp Vault token from File (#9644)
add be6ed86 Fixed failing Kubernetes tests after deny_all for
experimental API (#9647)
add 5e4b801 Test are triggered now on more changes. (#9646)
No new revisions were added by this update.
Summary of changes:
.github/workflows/ci.yml | 37 ++---
UPDATING.md| 4 +
.../api_connexion/endpoints/connection_endpoint.py | 7 +-
.../api_connexion/endpoints/dag_run_endpoint.py| 3 +-
.../api_connexion/endpoints/event_log_endpoint.py | 4 +
.../endpoints/import_error_endpoint.py | 11 +-
airflow/api_connexion/endpoints/log_endpoint.py| 2 +-
airflow/api_connexion/endpoints/pool_endpoint.py | 9 +-
.../api_connexion/endpoints/variable_endpoint.py | 4 +
airflow/api_connexion/endpoints/xcom_endpoint.py | 15 +-
airflow/api_connexion/openapi/v1.yaml | 1 -
airflow/api_connexion/parameters.py| 30 +++-
airflow/config_templates/airflow_local_settings.py | 4 +-
airflow/config_templates/config.yml| 16 ++
airflow/config_templates/default_airflow.cfg | 9 ++
airflow/exceptions.py | 2 +-
airflow/jobs/scheduler_job.py | 32 ++--
.../providers/amazon/aws/log}/__init__.py | 0
.../amazon/aws}/log/s3_task_handler.py | 0
.../operators/campaign_manager.py | 8 +-
.../marketing_platform/operators/display_video.py | 7 +
.../marketing_platform/operators/search_ads.py | 7 +
.../hashicorp/_internal_client/vault_client.py | 22 ++-
airflow/providers/hashicorp/hooks/vault.py | 5 +
airflow/providers/hashicorp/secrets/vault.py | 5 +
airflow/utils/log/es_task_handler.py | 34 +++-
airflow/utils/log/log_reader.py| 8 +-
airflow/utils/log/logging_mixin.py | 16 ++
airflow/utils/log/s3_task_handler.py | 177 ++---
airflow/www/templates/airflow/dag.html | 30 ++--
airflow/www/views.py | 55 +--
chart/README.md| 3 +-
chart/templates/configmap.yaml | 3 +
chart/values.yaml | 4 +
docs/autoapi_templates/index.rst | 13 ++
docs/howto/operator/gcp/campaign_manager.rst | 3 +-
docs/howto/operator/gcp/display_video.rst | 3 +-
docs/howto/operator/gcp/search_ads.rst | 3 +-
docs/howto/write-logs.rst | 26 +++
docs/index.rst | 3 +-
docs/project.rst | 2 +-
.../amazon/aws => stable-rest-api}/index.rst | 16 +-
scripts/ci/libraries/_kind.sh | 3 +-
.../endpoints/test_connection_endpoint.py | 31 +++-
.../endpoints/test_dag_run_endpoint.py | 14 +-
.../endpoints/test_event_log_endpoint.py | 14 +-
.../endpoints/test_import_error_endpoint.py| 36 -
tests/api_connexion/endpoints/test_log_endpoint.py | 2 +-
.../api_connexion/endpoints/test_pool_endpoint.py | 26 ++-
.../endpoints/test_variable_endpoint.py| 11 +-
tests/api_connexion/test_parameters.py | 39 -
tests/jobs/test_scheduler_job.py | 4 +-
.../{zendesk/hooks => amazon/aws/log}/__init__.py | 0
.../amazon/aws}/log/test_s3_task_handler.py| 6 +-
.../operators/test_campaign_manager.py | 19 +++
.../operators/test_display_video.py| 15 ++
.../operators/test_search_ads.py | 19 ++-
.../_internal_client/test_vault_client.py | 15 ++
tests/utils/log/test_es_task_handler.py| 22 +++
tests/www/test_views.py| 75 -
[GitHub] [airflow] ephraimbuddy opened a new pull request #9648: Update airflow to use the latest Flask App Builder
ephraimbuddy opened a new pull request #9648: URL: https://github.com/apache/airflow/pull/9648 --- Make sure to mark the boxes below before creating PR: [x] - [ ] Description above provides context of the change - [ ] Unit tests coverage for changes (not needed for documentation changes) - [ ] Target Github ISSUE in description if exists - [ ] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)" - [ ] Relevant documentation is updated including usage instructions. - [ ] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example). --- In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed. In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md). Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] Acehaidrey commented on pull request #9544: Add metric for scheduling delay between first run task & expected start time
Acehaidrey commented on pull request #9544: URL: https://github.com/apache/airflow/pull/9544#issuecomment-653705002 sorry to keep pinging @ashb , if you get a chance This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #9645: Get Airflow configs with sensitive data from Secret Backends
kaxil commented on a change in pull request #9645:
URL: https://github.com/apache/airflow/pull/9645#discussion_r449726296
##
File path: airflow/configuration.py
##
@@ -267,17 +271,49 @@ def _get_env_var_option(self, section, key):
env_var_cmd = env_var + '_CMD'
if env_var_cmd in os.environ:
# if this is a valid command key...
-if (section, key) in self.as_command_stdout:
+if (section, key) in self.configs_with_secret:
return run_command(os.environ[env_var_cmd])
def _get_cmd_option(self, section, key):
fallback_key = key + '_cmd'
# if this is a valid command key...
-if (section, key) in self.as_command_stdout:
+if (section, key) in self.configs_with_secret:
if super().has_option(section, fallback_key):
command = super().get(section, fallback_key)
return run_command(command)
+@cached_property
+def _secrets_backend_client(self):
+if super().has_option('secrets', 'backend') is False:
+return None
+
+secrets_backend_cls = super().get('secrets', 'backend')
+if not secrets_backend_cls:
+return None
+print("secrets_backend_cls", secrets_backend_cls)
+secrets_backend = import_string(secrets_backend_cls)
+if secrets_backend:
+try:
+alternative_secrets_config_dict = json.loads(
+super().get('secrets', 'backend_kwargs', fallback='{}')
+)
+except json.JSONDecodeError:
+alternative_secrets_config_dict = {}
+return secrets_backend(**alternative_secrets_config_dict)
Review comment:
Updated with TYPE_CHECKING in
https://github.com/apache/airflow/pull/9645/commits/ed8cc1a6316ddbd0971d1d5e56ca8c61bdab08e7
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #9645: Get Airflow configs with sensitive data from Secret Backends
kaxil commented on a change in pull request #9645:
URL: https://github.com/apache/airflow/pull/9645#discussion_r449726296
##
File path: airflow/configuration.py
##
@@ -267,17 +271,49 @@ def _get_env_var_option(self, section, key):
env_var_cmd = env_var + '_CMD'
if env_var_cmd in os.environ:
# if this is a valid command key...
-if (section, key) in self.as_command_stdout:
+if (section, key) in self.configs_with_secret:
return run_command(os.environ[env_var_cmd])
def _get_cmd_option(self, section, key):
fallback_key = key + '_cmd'
# if this is a valid command key...
-if (section, key) in self.as_command_stdout:
+if (section, key) in self.configs_with_secret:
if super().has_option(section, fallback_key):
command = super().get(section, fallback_key)
return run_command(command)
+@cached_property
+def _secrets_backend_client(self):
+if super().has_option('secrets', 'backend') is False:
+return None
+
+secrets_backend_cls = super().get('secrets', 'backend')
+if not secrets_backend_cls:
+return None
+print("secrets_backend_cls", secrets_backend_cls)
+secrets_backend = import_string(secrets_backend_cls)
+if secrets_backend:
+try:
+alternative_secrets_config_dict = json.loads(
+super().get('secrets', 'backend_kwargs', fallback='{}')
+)
+except json.JSONDecodeError:
+alternative_secrets_config_dict = {}
+return secrets_backend(**alternative_secrets_config_dict)
Review comment:
Updated with TYPE_CHECKING
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] houqp commented on pull request #9502: generate go client from openapi spec
houqp commented on pull request #9502: URL: https://github.com/apache/airflow/pull/9502#issuecomment-653702139 @potiuk moved codegen validation to separate workflow. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] vanka56 commented on a change in pull request #9472: Add drop_partition functionality for HiveMetastoreHook
vanka56 commented on a change in pull request #9472: URL: https://github.com/apache/airflow/pull/9472#discussion_r449715800 ## File path: tests/providers/apache/hive/hooks/test_hive.py ## @@ -383,6 +383,10 @@ def test_table_exists(self): self.hook.table_exists(str(random.randint(1, 1))) ) +def test_drop_partition(self): +self.assertTrue(self.hook.drop_partitions(self.table, db=self.database, + part_vals=[DEFAULT_DATE_DS])) + Review comment: @turbaszek Methods like def test_max_partition(self) also does the same. Moreover, it should not be an expensive operation. Do you think we really have to mock the call? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[airflow] branch master updated: Test are triggered now on more changes. (#9646)
This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/master by this push:
new 5e4b801 Test are triggered now on more changes. (#9646)
5e4b801 is described below
commit 5e4b801b32eeda79b59ff3cc8a3a503f57f5a509
Author: Jarek Potiuk
AuthorDate: Fri Jul 3 23:50:57 2020 +0200
Test are triggered now on more changes. (#9646)
Sometimes tests were not triggered when they should be.
This change will cause the tests to be triggered when anything
changes in "airflow" or "charts" additionally to what we had
before.
---
.github/workflows/ci.yml | 37 +++--
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 7eb80e9..f3b0e35 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -155,26 +155,27 @@ jobs:
run: |
./scripts/ci/ci_prepare_and_test_backport_packages.sh
- pyfiles:
+ trigger-tests:
timeout-minutes: 10
name: "Count changed important files"
runs-on: ubuntu-latest
outputs:
- count: ${{ steps.pyfiles.outputs.count }}
+ count: ${{ steps.trigger-tests.outputs.count }}
steps:
- uses: actions/checkout@master
- name: "Get count of changed python files"
run: |
set +e
./scripts/ci/ci_count_changed_files.sh ${GITHUB_SHA} \
- '\.py$|.github/workflows/|^Dockerfile|^scripts'
+ '^airflow|.github/workflows/|^Dockerfile|^scripts|^chart'
echo "::set-output name=count::$?"
-id: pyfiles
+id: trigger-tests
+
tests-kubernetes:
timeout-minutes: 80
name: "K8s: ${{matrix.kube-mode}} ${{matrix.python-version}}
${{matrix.kubernetes-version}}"
runs-on: ubuntu-latest
-needs: [static-checks-1, static-checks-2, pyfiles]
+needs: [static-checks-1, static-checks-2, trigger-tests]
strategy:
matrix:
python-version: [3.6, 3.7]
@@ -200,8 +201,8 @@ jobs:
KUBERNETES_VERSION: "${{ matrix.kubernetes-version }}"
KIND_VERSION: "${{ matrix.kind-version }}"
HELM_VERSION: "${{ matrix.helm-version }}"
-# For pull requests only run tests when python files changed
-if: needs.pyfiles.outputs.count != '0' || github.event_name !=
'pull_request'
+# For pull requests only run tests when important files changed
+if: needs.trigger-tests.outputs.count != '0' || github.event_name !=
'pull_request'
steps:
- uses: actions/checkout@master
- uses: actions/setup-python@v1
@@ -239,7 +240,7 @@ ${{
hashFiles('requirements/requirements-python${{matrix.python-version}}.txt')
timeout-minutes: 80
name:
"${{matrix.test-type}}:Pg${{matrix.postgres-version}},Py${{matrix.python-version}}"
runs-on: ubuntu-latest
-needs: [static-checks-1, static-checks-2, pyfiles]
+needs: [static-checks-1, static-checks-2, trigger-tests]
strategy:
matrix:
python-version: [3.6, 3.7, 3.8]
@@ -253,8 +254,8 @@ ${{
hashFiles('requirements/requirements-python${{matrix.python-version}}.txt')
RUN_TESTS: "true"
CI_JOB_TYPE: "Tests"
TEST_TYPE: ${{ matrix.test-type }}
-# For pull requests only run tests when python files changed
-if: needs.pyfiles.outputs.count != '0' || github.event_name !=
'pull_request'
+# For pull requests only run tests when important files changed
+if: needs.trigger-tests.outputs.count != '0' || github.event_name !=
'pull_request'
steps:
- uses: actions/checkout@master
- uses: actions/setup-python@v1
@@ -271,7 +272,7 @@ ${{
hashFiles('requirements/requirements-python${{matrix.python-version}}.txt')
timeout-minutes: 80
name: "${{matrix.test-type}}:MySQL${{matrix.mysql-version}},
Py${{matrix.python-version}}"
runs-on: ubuntu-latest
-needs: [static-checks-1, static-checks-2, pyfiles]
+needs: [static-checks-1, static-checks-2, trigger-tests]
strategy:
matrix:
python-version: [3.6, 3.7, 3.8]
@@ -285,8 +286,8 @@ ${{
hashFiles('requirements/requirements-python${{matrix.python-version}}.txt')
RUN_TESTS: "true"
CI_JOB_TYPE: "Tests"
TEST_TYPE: ${{ matrix.test-type }}
-# For pull requests only run tests when python files changed
-if: needs.pyfiles.outputs.count != '0' || github.event_name !=
'pull_request'
+# For pull requests only run tests when important files changed
+if: needs.trigger-tests.outputs.count != '0' || github.event_name !=
'pull_request'
steps:
- uses: actions/checkout@master
- uses: actions/setup-python@v1
@@ -303,7 +304,7 @@ ${{
hashFiles('requirements/requirements-python${{matrix.python-version}}.txt')
timeout-minutes: 80
name: "${{matrix.test-type}}:Sqlite Py${{matrix.python-version}}"
[GitHub] [airflow] potiuk merged pull request #9646: Test are triggered now on more changes.
potiuk merged pull request #9646: URL: https://github.com/apache/airflow/pull/9646 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] turbaszek commented on pull request #9531: Support .airflowignore for plugins
turbaszek commented on pull request #9531: URL: https://github.com/apache/airflow/pull/9531#issuecomment-653675958 @j-y-matsubara would you mind a rebase? The k8s tests should be fixed now This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] takunnithan commented on a change in pull request #9556: API Endpoint - DagRuns Batch
takunnithan commented on a change in pull request #9556: URL: https://github.com/apache/airflow/pull/9556#discussion_r449701317 ## File path: airflow/api_connexion/schemas/dag_run_schema.py ## @@ -80,15 +80,15 @@ class Meta: datetimeformat = 'iso' strict = True -page_offset = fields.Int(required=False, missing=0, min=0) -page_limit = fields.Int(required=False, missing=100, min=1) -dag_ids = fields.List(fields.Str(), required=False, missing=None) -execution_date_gte = fields.DateTime(required=False, missing=None) -execution_date_lte = fields.DateTime(required=False, missing=None) -start_date_gte = fields.DateTime(required=False, missing=None) -start_date_lte = fields.DateTime(required=False, missing=None) -end_date_gte = fields.DateTime(required=False, missing=None) -end_date_lte = fields.DateTime(required=False, missing=None) +page_offset = fields.Int(missing=0, min=0) +page_limit = fields.Int(missing=100, min=1) +dag_ids = fields.List(fields.Str(), missing=None) +execution_date_gte = fields.DateTime(missing=None) +execution_date_lte = fields.DateTime(missing=None) +start_date_gte = fields.DateTime(missing=None) +start_date_lte = fields.DateTime(missing=None) +end_date_gte = fields.DateTime(missing=None) +end_date_lte = fields.DateTime(missing=None) Review comment: The fields were missing in the deserialized data, when `missing=None` was removed. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #9556: API Endpoint - DagRuns Batch
ephraimbuddy commented on a change in pull request #9556: URL: https://github.com/apache/airflow/pull/9556#discussion_r449699768 ## File path: airflow/api_connexion/schemas/dag_run_schema.py ## @@ -80,15 +80,15 @@ class Meta: datetimeformat = 'iso' strict = True -page_offset = fields.Int(required=False, missing=0, min=0) -page_limit = fields.Int(required=False, missing=100, min=1) -dag_ids = fields.List(fields.Str(), required=False, missing=None) -execution_date_gte = fields.DateTime(required=False, missing=None) -execution_date_lte = fields.DateTime(required=False, missing=None) -start_date_gte = fields.DateTime(required=False, missing=None) -start_date_lte = fields.DateTime(required=False, missing=None) -end_date_gte = fields.DateTime(required=False, missing=None) -end_date_lte = fields.DateTime(required=False, missing=None) +page_offset = fields.Int(missing=0, min=0) +page_limit = fields.Int(missing=100, min=1) +dag_ids = fields.List(fields.Str(), missing=None) +execution_date_gte = fields.DateTime(missing=None) +execution_date_lte = fields.DateTime(missing=None) +start_date_gte = fields.DateTime(missing=None) +start_date_lte = fields.DateTime(missing=None) +end_date_gte = fields.DateTime(missing=None) +end_date_lte = fields.DateTime(missing=None) Review comment: Do you have any problem when you remove `missing=None`? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[airflow] branch master updated: Fixed failing Kubernetes tests after deny_all for experimental API (#9647)
This is an automated email from the ASF dual-hosted git repository.
ash pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/master by this push:
new be6ed86 Fixed failing Kubernetes tests after deny_all for
experimental API (#9647)
be6ed86 is described below
commit be6ed86ccd6e5921563be39877d93fb91713bbb9
Author: Jarek Potiuk
AuthorDate: Fri Jul 3 22:28:43 2020 +0200
Fixed failing Kubernetes tests after deny_all for experimental API (#9647)
The tests were broken by #9611
---
chart/templates/configmap.yaml | 3 +++
chart/values.yaml | 4
scripts/ci/libraries/_kind.sh | 3 ++-
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/chart/templates/configmap.yaml b/chart/templates/configmap.yaml
index 9a0c000..86affe8 100644
--- a/chart/templates/configmap.yaml
+++ b/chart/templates/configmap.yaml
@@ -42,6 +42,9 @@ data:
remote_logging = True
{{- end }}
+[api]
+auth_backend = {{ .Values.api.authBackend }}
+
[logging]
logging_level = DEBUG
[webserver]
diff --git a/chart/values.yaml b/chart/values.yaml
index 1918419..dc13064 100644
--- a/chart/values.yaml
+++ b/chart/values.yaml
@@ -434,3 +434,7 @@ postgresql:
enabled: true
postgresqlPassword: postgres
postgresqlUsername: postgres
+
+# Authentication backend used for the experimental API
+api:
+ authBackend: airflow.api.auth.backend.deny_all
diff --git a/scripts/ci/libraries/_kind.sh b/scripts/ci/libraries/_kind.sh
index 4f5ebe5..7b45b3a 100644
--- a/scripts/ci/libraries/_kind.sh
+++ b/scripts/ci/libraries/_kind.sh
@@ -294,7 +294,8 @@ function deploy_airflow_with_helm() {
--set "defaultAirflowRepository=${DOCKERHUB_USER}/${DOCKERHUB_REPO}" \
--set "images.airflow.repository=${DOCKERHUB_USER}/${DOCKERHUB_REPO}" \
--set "images.airflow.tag=${AIRFLOW_PROD_BASE_TAG}" -v 1 \
---set "defaultAirflowTag=${AIRFLOW_PROD_BASE_TAG}" -v 1
+--set "defaultAirflowTag=${AIRFLOW_PROD_BASE_TAG}" -v 1 \
+--set "api.authBackend=airflow.api.auth.backend.default"
echo
popd || exit 1
}
[GitHub] [airflow] ashb merged pull request #9647: Fixed failing Kubernetes tests after deny_all for experimental API
ashb merged pull request #9647: URL: https://github.com/apache/airflow/pull/9647 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #9647: Fixed failing Kubernetes tests after deny_all for experimental API
ashb commented on pull request #9647: URL: https://github.com/apache/airflow/pull/9647#issuecomment-653669532 Monorepo FTW. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] takunnithan commented on a change in pull request #9556: API Endpoint - DagRuns Batch
takunnithan commented on a change in pull request #9556:
URL: https://github.com/apache/airflow/pull/9556#discussion_r449696774
##
File path: tests/api_connexion/endpoints/test_dag_run_endpoint.py
##
@@ -346,6 +346,243 @@ def test_end_date_gte_lte(self, url,
expected_dag_run_ids, session):
self.assertEqual(dag_run_ids, expected_dag_run_ids)
+class TestGetDagRunBatch(TestDagRunEndpoint):
+@provide_session
+def test_should_respond_200(self, session):
+dag_runs = self._create_test_dag_run()
+session.add_all(dag_runs)
+session.commit()
+payload = {
+"dag_ids": ["TEST_DAG_ID"],
+}
+response = self.client.post("api/v1/dags/~/dagRuns/list", json=payload)
+assert response.status_code == 200
+self.assertEqual(
Review comment:
Yes. Replaced `assertEqual` with `assert`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] takunnithan commented on a change in pull request #9556: API Endpoint - DagRuns Batch
takunnithan commented on a change in pull request #9556: URL: https://github.com/apache/airflow/pull/9556#discussion_r449696638 ## File path: tests/api_connexion/endpoints/test_dag_run_endpoint.py ## @@ -346,6 +346,243 @@ def test_end_date_gte_lte(self, url, expected_dag_run_ids, session): self.assertEqual(dag_run_ids, expected_dag_run_ids) +class TestGetDagRunBatch(TestDagRunEndpoint): +@provide_session +def test_should_respond_200(self, session): +dag_runs = self._create_test_dag_run() +session.add_all(dag_runs) +session.commit() Review comment: Thanks @turbaszek . This is addressed in the latest commit. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] takunnithan commented on a change in pull request #9556: API Endpoint - DagRuns Batch
takunnithan commented on a change in pull request #9556: URL: https://github.com/apache/airflow/pull/9556#discussion_r449696389 ## File path: airflow/api_connexion/schemas/dag_run_schema.py ## @@ -72,5 +72,25 @@ class DAGRunCollectionSchema(Schema): total_entries = fields.Int() +class DagRunsBatchFormSchema(Schema): +""" Schema to validate and deserialize the Form(request payload) submitted to DagRun Batch endpoint""" + +class Meta: +""" Meta """ +datetimeformat = 'iso' +strict = True + +page_offset = fields.Int(required=False, missing=0, min=0) +page_limit = fields.Int(required=False, missing=100, min=1) +dag_ids = fields.List(fields.Str(), required=False, missing=None) +execution_date_gte = fields.DateTime(required=False, missing=None) Review comment: Thanks @ephraimbuddy . I have removed `required=False` from the args. But without `missing=None`, the fields were missing in deserialized data. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[jira] [Commented] (AIRFLOW-6786) Adding KafkaConsumerHook, KafkaProducerHook, and KafkaSensor
[ https://issues.apache.org/jira/browse/AIRFLOW-6786?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17150954#comment-17150954 ] ASF GitHub Bot commented on AIRFLOW-6786: - haidaraM commented on pull request #7407: URL: https://github.com/apache/airflow/pull/7407#issuecomment-653510340 Any news on this PR ? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Adding KafkaConsumerHook, KafkaProducerHook, and KafkaSensor > > > Key: AIRFLOW-6786 > URL: https://issues.apache.org/jira/browse/AIRFLOW-6786 > Project: Apache Airflow > Issue Type: New Feature > Components: contrib, hooks >Affects Versions: 1.10.9 >Reporter: Daniel Ferguson >Assignee: Daniel Ferguson >Priority: Minor > > Add the KafkaProducerHook. > Add the KafkaConsumerHook. > Add the KafkaSensor which listens to messages with a specific topic. > Related Issue: > #1311 (Pre-dates Jira Migration) > Reminder to contributors: > You must add an Apache License header to all new files > Please squash your commits when possible and follow the 7 rules of good Git > commits > I am new to the community, I am not sure the files are at the right place or > missing anything. > The sensor could be used as the first node of a dag where the second node can > be a TriggerDagRunOperator. The messages are polled in a batch and the dag > runs are dynamically generated. > Thanks! > Note, as per denied PR [#1415|https://github.com/apache/airflow/pull/1415], > it is important to mention these integrations are not suitable for > low-latency/high-throughput/streaming. For reference, [#1415 > (comment)|https://github.com/apache/airflow/pull/1415#issuecomment-484429806]. > Co-authored-by: Dan Ferguson > [dferguson...@gmail.com|mailto:dferguson...@gmail.com] > Co-authored-by: YuanfΞi Zhu -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (AIRFLOW-3515) Remove the run_duration option
[ https://issues.apache.org/jira/browse/AIRFLOW-3515?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17150812#comment-17150812 ] ASF GitHub Bot commented on AIRFLOW-3515: - Fokko commented on pull request #4320: URL: https://github.com/apache/airflow/pull/4320#issuecomment-653407133 @shargal Restarting the scheduler isn't really a proper solution, right? Feels a bit like; have you tried turning it off and on again. Would be cool if we could isolate the problem of why the tasks get stuck or add logic to check which tasks are stuck and retry them. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Remove the run_duration option > -- > > Key: AIRFLOW-3515 > URL: https://issues.apache.org/jira/browse/AIRFLOW-3515 > Project: Apache Airflow > Issue Type: Task >Reporter: Fokko Driesprong >Priority: Major > Fix For: 2.0.0 > > > We should not use the `run_duration` option anymore. This used to be for > restarting the scheduler from time to time, but right now the scheduler is > getting more stable and therefore using this setting is considered bad and > might cause an inconsistent state. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [airflow] kaxil edited a comment on pull request #9647: Fixed failing Kubernetes tests after deny_all for experimental API
kaxil edited a comment on pull request #9647: URL: https://github.com/apache/airflow/pull/9647#issuecomment-653649541 > Please do. I think it's the "eat cake and have it too" case. We can fulfill all your expectations (separate releases, issues for users in separate repos) while keeping much lower complexity of the development process. Yup ignore my last comment, I will definitely give the k8s approach a good look and get back on the discussion :) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #9647: Fixed failing Kubernetes tests after deny_all for experimental API
kaxil commented on pull request #9647: URL: https://github.com/apache/airflow/pull/9647#issuecomment-653649541 > Please do. I think it's the "eat cake and have it too" case. We can fulfill all your expectations (separate releases, issues for users in separate repos) while keeping much lower complexity of the development process. Yup definitely :) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #9647: Fixed failing Kubernetes tests after deny_all for experimental API
kaxil commented on pull request #9647: URL: https://github.com/apache/airflow/pull/9647#issuecomment-653648597 > > On the flip side changes in the Helm Chart should not affect Airflow's CI. In this case, the default of disabling API should just be a change in Airflow and should not be a change in the Helm chart version until a new Airflow version is released. > > _Should_. This is a nice idea, but until we are really stable and have all the details worked out for helm chart and dockerfile there will be hiden couplings - even beyond that. IMHO this is the same fallacy as with micro-services hype. With Micro-services in theory you have decoupled services that can be developed in isolation but in fact a lot of micro-services have hidden couplings and what you gain with separation, you loose on coordination. A lot of teams (especially those not huge ones) withdraw from micro-services approach (I'd say hype) recently, precisely because it is not full-filing the promises (i.e. for smaller teams costs of coordination are far bigger than benefits of isolation). It's never 0-1, it's always cost-gain game. > > I believe we are still far to small (both code-wise and people-wise) and the "chart" and "dockerfile" are not big enough on it's own to get enough isolation gains to cover the separation cost. > > > Btw I am not against the Kubernetes way, I will look into the details and let you'll know on the thread. But as of now I am still on the "separate repo" side > > Please do. I think it's the "eat cake and have it too" case. We can fulfill all your expectations (separate releases, issues for users in separate repos) while keeping much lower complexity of the development process. I would say we should not use the Helm Chart for test until it gets stable then. WDYT? Let's work on getting the Helm Chart to the ideal stable before using it for tests. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #9647: Fixed failing Kubernetes tests after deny_all for experimental API
potiuk edited a comment on pull request #9647: URL: https://github.com/apache/airflow/pull/9647#issuecomment-653646797 > On the flip side changes in the Helm Chart should not affect Airflow's CI. In this case, the default of disabling API should just be a change in Airflow and should not be a change in the Helm chart version until a new Airflow version is released. *Should*. This is a nice idea, but until we are really stable and have all the details worked out for helm chart and dockerfile there will be hiden couplings - even beyond that. IMHO this is the same fallacy as with micro-services hype. With Micro-services in theory you have decoupled services that can be developed in isolation but in fact a lot of micro-services have hidden couplings and what you gain with separation, you loose on coordination. A lot of teams (especially those not huge ones) withdraw from micro-services approach (I'd say hype) recently, precisely because it is not full-filing the promises (i.e. for smaller teams costs of coordination are far bigger than benefits of isolation). It's never 0-1, it's always cost-gain game. I believe we are still far to small (both code-wise and people-wise) and the "chart" and "dockerfile" are not big enough on it's own to get enough isolation gains to cover the separation cost. > Btw I am not against the Kubernetes way, I will look into the details and let you'll know on the thread. But as of now I am still on the "separate repo" side Please do. I think it's the "eat cake and have it too" case. We can fulfill all your expectations (separate releases, issues for users in separate repos) while keeping much lower complexity of the development process. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #9647: Fixed failing Kubernetes tests after deny_all for experimental API
potiuk commented on pull request #9647: URL: https://github.com/apache/airflow/pull/9647#issuecomment-653646797 > On the flip side changes in the Helm Chart should not affect Airflow's CI. In this case, the default of disabling API should just be a change in Airflow and should not be a change in the Helm chart version until a new Airflow version is released. *Should*. This is a nice idea, but until we are really stable and have all the details worked out for helm chart and dockerfile there will be hiden couplings - even beyond that. IMHO this is the same fallacy as with micro-services hype. With Micro-services in theory you have decoupled services that can be developed in isolation but in fact a lot of micro-services have hidden couplings and what you gain with separation, you loose on coordination. A lot of teams (especially those not huge ones) withdraw from micro-services approach (I'd say hype) recently, precisely because it is not full-filing the promises (i.e. for smaller teams costs of coordination are far bigger than benefits of isolation). It's never 0-1, it's always cost-gain game. I believe we are still far to small (both code-wise and people-wise) and the "chart" and "dockerfile" are not big enough on it's own to get enough isolation benefits to cover the separation gain. > Btw I am not against the Kubernetes way, I will look into the details and let you'll know on the thread. But as of now I am still on the "separate repo" side Please do. I think it's the "eat cake and have it too" case. We can fulfill all your expectations (separate releases, issues for users in separate repos) while keeping much lower complexity of the development process. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow-client-go] houqp commented on pull request #1: Add generated go client
houqp commented on pull request #1: URL: https://github.com/apache/airflow-client-go/pull/1#issuecomment-653646372 @turbaszek added license header to ci config. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #9647: Fixed failing Kubernetes tests after deny_all for experimental API
kaxil commented on pull request #9647: URL: https://github.com/apache/airflow/pull/9647#issuecomment-653644916 On the flip side changes in the Helm Chart should not affect Airflow's CI. In this case, the default of disabling API should just be a change in Airflow and should not be a change in the Helm chart version until a new Airflow version is released. > Having a dedicated config is much nicer solution. And if we were to do this for split repos we would have to implement "workaround" first, "good solution" in a chart repo, release the repo and finally implement a "good" solution. I am sure it is not worth it and I love how kubernetes team solved it. We already have a dedicated config `default_test.cfg` (https://github.com/apache/airflow/blob/master/airflow/config_templates/default_test.cfg) we should be using that for tests. I don't see a need of "workaround" here? Btw I am not against the Kubernetes way, I will look into the details and let you'll know on the thread. But as of now I am still on the "separate repo" side This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #9647: Fixed failing Kubernetes tests after deny_all for experimental API
potiuk commented on pull request #9647: URL: https://github.com/apache/airflow/pull/9647#issuecomment-653643945 Having a dedicated config is much nicer solution. And if we were to do this for split repos we would have to implement "workaround" first, "good solution" in a chart repo, release the repo and finally implement a "good" solution. I am sure it is not worth it and I love how kubernetes team solved 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #9647: Fixed failing Kubernetes tests after deny_all for experimental API
kaxil commented on pull request #9647: URL: https://github.com/apache/airflow/pull/9647#issuecomment-653643433 But why not use Environment Variables to set configs. That's what we want to allow users to do too with the Chart, they would like to override any config. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #9647: Fixed failing Kubernetes tests after deny_all for experimental API
potiuk commented on pull request #9647: URL: https://github.com/apache/airflow/pull/9647#issuecomment-653643245 > Well you could just set up Environment variable, here > > https://github.com/apache/airflow/blob/master/chart/values.yaml#L94-L98 > > So theory and in-practise here are not different But this could be much more complex change needed to make this works - this is really one liner that shows how simple it is now to fix those kind of problems and how complex it could be if we split. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #9647: Fixed failing Kubernetes tests after deny_all for experimental API
kaxil commented on pull request #9647: URL: https://github.com/apache/airflow/pull/9647#issuecomment-653642913 Well you could just set up Environment variable, here https://github.com/apache/airflow/blob/master/chart/values.yaml#L94-L98 So theory and in-practise here are not different This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #9647: Fixed failing Kubernetes tests after deny_all for experimental API
potiuk commented on pull request #9647: URL: https://github.com/apache/airflow/pull/9647#issuecomment-653642786 > I think it should still work. In theory, the chart should allow overriding any runtime Airflow configs. In this case we could have used Environment Variable to set config. If we are not able to do we would anyway hit that limitation In theory, theory and practice are the same. In practice, they are 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] kaxil edited a comment on pull request #9647: Fixed failing Kubernetes tests after deny_all for experimental API
kaxil edited a comment on pull request #9647: URL: https://github.com/apache/airflow/pull/9647#issuecomment-653642379 > Side note > > @kaxil @dimberman @ashb @turbaszek @mik-laj - this is an example of where splitting to separate repo would have been problematic. > > This change should have been committed together with #9611 and it would have been rather complex to test if the `chart` and `airflow` were separate repos. It would not work with the scheme proposed by @kaxil where we only use "released" versions of helm chart for Airflow tests and the other way round because in this case changing the backend auth to "deny_all" had to be coupled with adding new configuration value in the helm chart. > > It could work with the submodule scheme I proposed but as discussed with @dimberman yesterday - just coordinating this simple change across two PRs done in two different repos would have been a much more complex task. > > And this is just one small change ... > > I really think that monorepo approach and possibly split-out to separate repos with pushing only related changes (like @ryw commented) could be a much better solution. I think it should still work. In theory, the chart should allow overriding any runtime Airflow configs. In this case we could have used Environment Variable to set config. If we are not able to do we would anyway hit that limitation. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] kaxil edited a comment on pull request #9647: Fixed failing Kubernetes tests after deny_all for experimental API
kaxil edited a comment on pull request #9647: URL: https://github.com/apache/airflow/pull/9647#issuecomment-653642379 > Side note > > @kaxil @dimberman @ashb @turbaszek @mik-laj - this is an example of where splitting to separate repo would have been problematic. > > This change should have been committed together with #9611 and it would have been rather complex to test if the `chart` and `airflow` were separate repos. It would not work with the scheme proposed by @kaxil where we only use "released" versions of helm chart for Airflow tests and the other way round because in this case changing the backend auth to "deny_all" had to be coupled with adding new configuration value in the helm chart. > > It could work with the submodule scheme I proposed but as discussed with @dimberman yesterday - just coordinating this simple change across two PRs done in two different repos would have been a much more complex task. > > And this is just one small change ... > > I really think that monorepo approach and possibly split-out to separate repos with pushing only related changes (like @ryw commented) could be a much better solution. I think it should still work. In theory, the chart should allow overriding any runtime Airflow configs. In this case we could have used Environment Variable to set config. If we are not able to do we would anyway hit that limitation This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #9647: Fixed failing Kubernetes tests after deny_all for experimental API
kaxil commented on pull request #9647: URL: https://github.com/apache/airflow/pull/9647#issuecomment-653642379 > Side note > > @kaxil @dimberman @ashb @turbaszek @mik-laj - this is an example of where splitting to separate repo would have been problematic. > > This change should have been committed together with #9611 and it would have been rather complex to test if the `chart` and `airflow` were separate repos. It would not work with the scheme proposed by @kaxil where we only use "released" versions of helm chart for Airflow tests and the other way round because in this case changing the backend auth to "deny_all" had to be coupled with adding new configuration value in the helm chart. > > It could work with the submodule scheme I proposed but as discussed with @dimberman yesterday - just coordinating this simple change across two PRs done in two different repos would have been a much more complex task. > > And this is just one small change ... > > I really think that monorepo approach and possibly split-out to separate repos with pushing only related changes (like @ryw commented) could be a much better solution. I think it should still work. If it doesn't the chart is not robust enough. In theory, the chart should allow overriding any runtime Airflow configs. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #9647: Fixed failing Kubernetes tests after deny_all for experimental API
potiuk edited a comment on pull request #9647: URL: https://github.com/apache/airflow/pull/9647#issuecomment-653639691 Side note @kaxil @dimberman @ashb @turbaszek @mik-laj - this is an example of where splitting to separate repo would have been problematic. This change should have been committed together with #9611 and it would have been rather complex to test if the `chart` and `airflow` were separate repos. It would not work with the scheme proposed by @kaxil where we only use "released" versions of helm chart for Airflow tests and the other way round because in this case changing the backend auth to "deny_all" had to be coupled with adding new configuration value in the helm chart. It could work with the submodule scheme I proposed but as discussed with @dimberman yesterday - just coordinating this simple change across two PRs done in two different repos would have been a much more complex task. And this is just one small change ... I really think that monorepo approach and possibly split-out to separate repos with pushing only related changes (like @ryw commented) could be a much better solution. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #9647: Fixed failing Kubernetes tests after deny_all for experimental API
potiuk commented on pull request #9647: URL: https://github.com/apache/airflow/pull/9647#issuecomment-653639691 Side note @kaxil @dimberman @ashb @turbaszek @mik-laj - this is an example of where splitting to separate repo would have been problematic. This change should have been committed together with #9611 and it would have been rather complex to test if the helm chart and kubernetes were separate repos. It would not work with the scheme proposed by @kaxil where we only use "released" versions of helm chart for Airflow tests and the other way round because in this case changing the backend auth to "deny_all" had to be coupled with adding new configuration value in the helm chart. It could work with the submodule scheme I proposed but as discussed with @dimberman yesterday - just coordinating this simple change across two PRs done in two different repos would have been a much more complex task. And this is just one small change ... I really think that monorepo approach and possibly split-out to separate repos with pushing only related changes (like @ryw commented) could be a much better solution. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk opened a new pull request #9647: Fixed failing Kubernetes tests after deny_all for experimental API
potiuk opened a new pull request #9647: URL: https://github.com/apache/airflow/pull/9647 --- Make sure to mark the boxes below before creating PR: [x] - [x] Description above provides context of the change - [x] Unit tests coverage for changes (not needed for documentation changes) - [x] Target Github ISSUE in description if exists - [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)" - [x] Relevant documentation is updated including usage instructions. - [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example). --- In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed. In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md). Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk opened a new pull request #9646: Test are triggered now on more changes.
potiuk opened a new pull request #9646: URL: https://github.com/apache/airflow/pull/9646 --- Make sure to mark the boxes below before creating PR: [x] - [x] Description above provides context of the change - [x] Unit tests coverage for changes (not needed for documentation changes) - [x] Target Github ISSUE in description if exists - [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)" - [x] Relevant documentation is updated including usage instructions. - [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example). --- In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed. In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md). Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] j-y-matsubara commented on a change in pull request #9531: Support .airflowignore for plugins
j-y-matsubara commented on a change in pull request #9531:
URL: https://github.com/apache/airflow/pull/9531#discussion_r449667794
##
File path: tests/plugins/test_plugin_ignore.py
##
@@ -0,0 +1,96 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+import os
+import shutil
+import tempfile
+import unittest
+from unittest.mock import patch
+
+from airflow import settings # type: ignore
+from airflow.utils.file import find_path_from_directory # type: ignore
+
+
+class TestIgnorePluginFile(unittest.TestCase):
+"""
+Test that the .airflowignore work and whether the file is properly ignored.
+"""
+
+def setUp(self):
+"""
+Make tmp folder and files that should be ignored. And set base path.
+"""
+self.test_dir = tempfile.mkdtemp()
+self.test_file = os.path.join(self.test_dir, 'test_file.txt')
+self.plugin_folder_path = os.path.join(self.test_dir, 'test_ignore')
+os.mkdir(os.path.join(self.test_dir, "test_ignore"))
+with open(os.path.join(self.plugin_folder_path, "test_load.py"), "w")
as file:
+file.write("#Should not be ignored file")
+with open(os.path.join(self.plugin_folder_path, ".airflowignore"),
"w") as file:
+file.write("#ignore test\nnot\nsubdir2")
+os.mkdir(os.path.join(self.plugin_folder_path, "subdir1"))
+with open(os.path.join(self.plugin_folder_path,
"subdir1/.airflowignore"), "w") as file:
+file.write("#ignore test\nnone")
+with open(os.path.join(self.plugin_folder_path,
"subdir1/test_load_sub1.py"), "w") as file:
+file.write("#Should not be ignored file")
+with open(os.path.join(self.plugin_folder_path,
"test_notload_sub.py"), 'w') as file:
+file.write('raise Exception("This file should have been
ignored!")')
+with open(os.path.join(self.plugin_folder_path,
"subdir1/test_noneload_sub1.py"), 'w') as file:
+file.write('raise Exception("This file should have been
ignored!")')
+os.mkdir(os.path.join(self.plugin_folder_path, "subdir2"))
+with open(os.path.join(self.plugin_folder_path,
"subdir2/test_shouldignore.py"), 'w') as file:
+file.write('raise Exception("This file should have been
ignored!")')
+with open(os.path.join(self.plugin_folder_path,
"subdir2/test_shouldignore.py"), 'w') as file:
+file.write('raise Exception("This file should have been
ignored!")')
Review comment:
Yes!
The notation you suggest is better than the existing my code.
I fixed.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[airflow] branch master updated (fddc572 -> a99aaeb)
This is an automated email from the ASF dual-hosted git repository. kaxilnaik pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/airflow.git. from fddc572 Customizable page size limit in API (#9431) add a99aaeb Allow setting Hashicorp Vault token from File (#9644) No new revisions were added by this update. Summary of changes: .../hashicorp/_internal_client/vault_client.py | 22 +- airflow/providers/hashicorp/hooks/vault.py | 5 + airflow/providers/hashicorp/secrets/vault.py | 5 + .../_internal_client/test_vault_client.py | 15 +++ 4 files changed, 42 insertions(+), 5 deletions(-)
[airflow] branch master updated (fddc572 -> a99aaeb)
This is an automated email from the ASF dual-hosted git repository. kaxilnaik pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/airflow.git. from fddc572 Customizable page size limit in API (#9431) add a99aaeb Allow setting Hashicorp Vault token from File (#9644) No new revisions were added by this update. Summary of changes: .../hashicorp/_internal_client/vault_client.py | 22 +- airflow/providers/hashicorp/hooks/vault.py | 5 + airflow/providers/hashicorp/secrets/vault.py | 5 + .../_internal_client/test_vault_client.py | 15 +++ 4 files changed, 42 insertions(+), 5 deletions(-)
[GitHub] [airflow] kaxil merged pull request #9644: Allow setting Hashicorp Vault token from File
kaxil merged pull request #9644: URL: https://github.com/apache/airflow/pull/9644 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #9645: Get Airflow configs with sensitive data from Secret Backends
kaxil commented on a change in pull request #9645:
URL: https://github.com/apache/airflow/pull/9645#discussion_r449663507
##
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##
@@ -62,13 +67,15 @@ def __init__(
self,
connections_prefix: str = 'airflow/connections',
variables_prefix: str = 'airflow/variables',
+configurations_prefix: str = 'airflow/configuration',
Review comment:
fixed
##
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##
@@ -42,8 +42,11 @@ class SecretsManagerBackend(BaseSecretsBackend,
LoggingMixin):
For example, if secrets prefix is ``airflow/connections/smtp_default``,
this would be accessible
if you provide ``{"connections_prefix": "airflow/connections"}`` and
request conn_id ``smtp_default``.
-And if variables prefix is ``airflow/variables/hello``, this would be
accessible
+If variables prefix is ``airflow/variables/hello``, this would be
accessible
if you provide ``{"variables_prefix": "airflow/variables"}`` and request
variable key ``hello``.
+And if configurations_prefix is
``airflow/configurations/sql_alchemy_conn``, this would be accessible
+if you provide ``{"configurations_prefix": "airflow/configurations"}`` and
request variable
Review comment:
fixed
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #9645: Get Airflow configs with sensitive data from Secret Backends
kaxil commented on a change in pull request #9645:
URL: https://github.com/apache/airflow/pull/9645#discussion_r449663361
##
File path: airflow/configuration.py
##
@@ -267,17 +271,49 @@ def _get_env_var_option(self, section, key):
env_var_cmd = env_var + '_CMD'
if env_var_cmd in os.environ:
# if this is a valid command key...
-if (section, key) in self.as_command_stdout:
+if (section, key) in self.configs_with_secret:
return run_command(os.environ[env_var_cmd])
def _get_cmd_option(self, section, key):
fallback_key = key + '_cmd'
# if this is a valid command key...
-if (section, key) in self.as_command_stdout:
+if (section, key) in self.configs_with_secret:
if super().has_option(section, fallback_key):
command = super().get(section, fallback_key)
return run_command(command)
+@cached_property
+def _secrets_backend_client(self):
+if super().has_option('secrets', 'backend') is False:
+return None
+
+secrets_backend_cls = super().get('secrets', 'backend')
+if not secrets_backend_cls:
+return None
+print("secrets_backend_cls", secrets_backend_cls)
+secrets_backend = import_string(secrets_backend_cls)
+if secrets_backend:
+try:
+alternative_secrets_config_dict = json.loads(
+super().get('secrets', 'backend_kwargs', fallback='{}')
+)
+except json.JSONDecodeError:
+alternative_secrets_config_dict = {}
+return secrets_backend(**alternative_secrets_config_dict)
Review comment:
```ini
sql_alchemy_conn_secret = conn/sql_alchemy_conn
```
should already allow that.
Whatever they pass to `sql_alchemy_conn_secret` will be directly used to get
secret
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #9645: Get Airflow configs with sensitive data from Secret Backends
kaxil commented on a change in pull request #9645:
URL: https://github.com/apache/airflow/pull/9645#discussion_r449648643
##
File path: airflow/configuration.py
##
@@ -267,17 +271,49 @@ def _get_env_var_option(self, section, key):
env_var_cmd = env_var + '_CMD'
if env_var_cmd in os.environ:
# if this is a valid command key...
-if (section, key) in self.as_command_stdout:
+if (section, key) in self.configs_with_secret:
return run_command(os.environ[env_var_cmd])
def _get_cmd_option(self, section, key):
fallback_key = key + '_cmd'
# if this is a valid command key...
-if (section, key) in self.as_command_stdout:
+if (section, key) in self.configs_with_secret:
if super().has_option(section, fallback_key):
command = super().get(section, fallback_key)
return run_command(command)
+@cached_property
+def _secrets_backend_client(self):
+if super().has_option('secrets', 'backend') is False:
+return None
+
+secrets_backend_cls = super().get('secrets', 'backend')
+if not secrets_backend_cls:
+return None
+print("secrets_backend_cls", secrets_backend_cls)
+secrets_backend = import_string(secrets_backend_cls)
+if secrets_backend:
+try:
+alternative_secrets_config_dict = json.loads(
+super().get('secrets', 'backend_kwargs', fallback='{}')
+)
+except json.JSONDecodeError:
+alternative_secrets_config_dict = {}
+return secrets_backend(**alternative_secrets_config_dict)
Review comment:
The main reason there is some code duplication here is we can't use
`airflow.secrets` as otherwise we would have infinite loop (even when the
import is inside the loop) since the DEFAULT_SECRET_BACKEND ( Environment
Variable and Connection ) needs `sql_achemy_conn` value.
I can separate the code to a different file and import that file at both
places if the concern is just duplication
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #9645: Get Airflow configs with sensitive data from Secret Backends
kaxil commented on a change in pull request #9645:
URL: https://github.com/apache/airflow/pull/9645#discussion_r449648643
##
File path: airflow/configuration.py
##
@@ -267,17 +271,49 @@ def _get_env_var_option(self, section, key):
env_var_cmd = env_var + '_CMD'
if env_var_cmd in os.environ:
# if this is a valid command key...
-if (section, key) in self.as_command_stdout:
+if (section, key) in self.configs_with_secret:
return run_command(os.environ[env_var_cmd])
def _get_cmd_option(self, section, key):
fallback_key = key + '_cmd'
# if this is a valid command key...
-if (section, key) in self.as_command_stdout:
+if (section, key) in self.configs_with_secret:
if super().has_option(section, fallback_key):
command = super().get(section, fallback_key)
return run_command(command)
+@cached_property
+def _secrets_backend_client(self):
+if super().has_option('secrets', 'backend') is False:
+return None
+
+secrets_backend_cls = super().get('secrets', 'backend')
+if not secrets_backend_cls:
+return None
+print("secrets_backend_cls", secrets_backend_cls)
+secrets_backend = import_string(secrets_backend_cls)
+if secrets_backend:
+try:
+alternative_secrets_config_dict = json.loads(
+super().get('secrets', 'backend_kwargs', fallback='{}')
+)
+except json.JSONDecodeError:
+alternative_secrets_config_dict = {}
+return secrets_backend(**alternative_secrets_config_dict)
Review comment:
The main reason there is some code duplication here is we can't use
`airflow.secrets` as otherwise we would have infinite loop since the
DEFAULT_SECRET_BACKEND ( Environment Variable and Connection ) needs
`sql_achemy_conn` value.
I can separate the code to a different file and import that file at both
places if the concern is just duplication
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #9645: Get Airflow configs with sensitive data from Secret Backends
kaxil commented on a change in pull request #9645:
URL: https://github.com/apache/airflow/pull/9645#discussion_r449648643
##
File path: airflow/configuration.py
##
@@ -267,17 +271,49 @@ def _get_env_var_option(self, section, key):
env_var_cmd = env_var + '_CMD'
if env_var_cmd in os.environ:
# if this is a valid command key...
-if (section, key) in self.as_command_stdout:
+if (section, key) in self.configs_with_secret:
return run_command(os.environ[env_var_cmd])
def _get_cmd_option(self, section, key):
fallback_key = key + '_cmd'
# if this is a valid command key...
-if (section, key) in self.as_command_stdout:
+if (section, key) in self.configs_with_secret:
if super().has_option(section, fallback_key):
command = super().get(section, fallback_key)
return run_command(command)
+@cached_property
+def _secrets_backend_client(self):
+if super().has_option('secrets', 'backend') is False:
+return None
+
+secrets_backend_cls = super().get('secrets', 'backend')
+if not secrets_backend_cls:
+return None
+print("secrets_backend_cls", secrets_backend_cls)
+secrets_backend = import_string(secrets_backend_cls)
+if secrets_backend:
+try:
+alternative_secrets_config_dict = json.loads(
+super().get('secrets', 'backend_kwargs', fallback='{}')
+)
+except json.JSONDecodeError:
+alternative_secrets_config_dict = {}
+return secrets_backend(**alternative_secrets_config_dict)
Review comment:
The main there is some code duplication here is we can't use
`airflow.secrets` as otherwise we would have infinite loop since the
DEFAULT_SECRET_BACKEND ( Environment Variable and Connection ) needs
`sql_achemy_conn` value
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #9645: Get Airflow configs with sensitive data from Secret Backends
kaxil commented on a change in pull request #9645: URL: https://github.com/apache/airflow/pull/9645#discussion_r449647991 ## File path: airflow/providers/amazon/aws/secrets/secrets_manager.py ## @@ -62,13 +67,15 @@ def __init__( self, connections_prefix: str = 'airflow/connections', variables_prefix: str = 'airflow/variables', +configurations_prefix: str = 'airflow/configuration', Review comment: :D I went back and forth with this This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #9645: Get Airflow configs with sensitive data from Secret Backends
ashb commented on a change in pull request #9645:
URL: https://github.com/apache/airflow/pull/9645#discussion_r449642855
##
File path: airflow/configuration.py
##
@@ -115,7 +117,9 @@ class AirflowConfigParser(ConfigParser):
# These configuration elements can be fetched as the stdout of commands
# following the "{section}__{name}__cmd" pattern, the idea behind this
# is to not store password on boxes in text files.
-as_command_stdout = {
+# These configs can also be fetched from Secrets backend
+# following the "{section}__{name}__secret" pattern
+configs_with_secret = {
Review comment:
```suggestion
senstive_config_values = {
```
##
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##
@@ -62,13 +67,15 @@ def __init__(
self,
connections_prefix: str = 'airflow/connections',
variables_prefix: str = 'airflow/variables',
+configurations_prefix: str = 'airflow/configuration',
Review comment:
```suggestion
config_prefix: str = 'airflow/config',
```
##
File path: airflow/secrets/base_secrets.py
##
@@ -73,3 +73,12 @@ def get_variable(self, key: str) -> Optional[str]:
:return: Variable Value
"""
raise NotImplementedError()
+
+def get_configuration(self, key: str) -> Optional[str]:
Review comment:
```suggestion
def get_configuration(self, section: str, key: str) -> Optional[str]:
```
##
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##
@@ -62,13 +67,15 @@ def __init__(
self,
connections_prefix: str = 'airflow/connections',
variables_prefix: str = 'airflow/variables',
+configurations_prefix: str = 'airflow/configuration',
Review comment:
etc (haven't made this change everywhere)
##
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##
@@ -42,8 +42,11 @@ class SecretsManagerBackend(BaseSecretsBackend,
LoggingMixin):
For example, if secrets prefix is ``airflow/connections/smtp_default``,
this would be accessible
if you provide ``{"connections_prefix": "airflow/connections"}`` and
request conn_id ``smtp_default``.
-And if variables prefix is ``airflow/variables/hello``, this would be
accessible
+If variables prefix is ``airflow/variables/hello``, this would be
accessible
if you provide ``{"variables_prefix": "airflow/variables"}`` and request
variable key ``hello``.
+And if configurations_prefix is
``airflow/configurations/sql_alchemy_conn``, this would be accessible
Review comment:
```suggestion
And if configurations_prefix is
``airflow/config/core/sql_alchemy_conn``, this would be accessible
```
##
File path: airflow/configuration.py
##
@@ -267,17 +271,49 @@ def _get_env_var_option(self, section, key):
env_var_cmd = env_var + '_CMD'
if env_var_cmd in os.environ:
# if this is a valid command key...
-if (section, key) in self.as_command_stdout:
+if (section, key) in self.configs_with_secret:
return run_command(os.environ[env_var_cmd])
def _get_cmd_option(self, section, key):
fallback_key = key + '_cmd'
# if this is a valid command key...
-if (section, key) in self.as_command_stdout:
+if (section, key) in self.configs_with_secret:
if super().has_option(section, fallback_key):
command = super().get(section, fallback_key)
return run_command(command)
+@cached_property
+def _secrets_backend_client(self):
+if super().has_option('secrets', 'backend') is False:
+return None
+
+secrets_backend_cls = super().get('secrets', 'backend')
+if not secrets_backend_cls:
+return None
+print("secrets_backend_cls", secrets_backend_cls)
+secrets_backend = import_string(secrets_backend_cls)
+if secrets_backend:
+try:
+alternative_secrets_config_dict = json.loads(
+super().get('secrets', 'backend_kwargs', fallback='{}')
+)
+except json.JSONDecodeError:
+alternative_secrets_config_dict = {}
+return secrets_backend(**alternative_secrets_config_dict)
Review comment:
H, not a huge fan of the duplication here and in
`airflow/secrets/__init__.py`.
Since this would only be called _after_ the class is constructed would we
instead do
```suggestion
@cached_property
def _secrets_backend_client(self):
if super().has_option('secrets', 'backend') is False:
return None
from airflow.secrets import secrets_backend_list
return secrets_backend_list
```
(give or take).
By delaying the import to inside the
[GitHub] [airflow] kaxil commented on a change in pull request #9645: Get Airflow configs with sensitive data from Secret Backends
kaxil commented on a change in pull request #9645:
URL: https://github.com/apache/airflow/pull/9645#discussion_r449642561
##
File path: airflow/configuration.py
##
@@ -267,17 +272,49 @@ def _get_env_var_option(self, section, key):
env_var_cmd = env_var + '_CMD'
if env_var_cmd in os.environ:
# if this is a valid command key...
-if (section, key) in self.as_command_stdout:
+if (section, key) in self.configs_with_secret:
return run_command(os.environ[env_var_cmd])
def _get_cmd_option(self, section, key):
fallback_key = key + '_cmd'
# if this is a valid command key...
-if (section, key) in self.as_command_stdout:
+if (section, key) in self.configs_with_secret:
if super().has_option(section, fallback_key):
command = super().get(section, fallback_key)
return run_command(command)
+@cached_property
+def _secrets_backend_client(self):
+if super().has_option('secrets', 'backend') is False:
+return None
+
+secrets_backend_cls = super().get('secrets', 'backend')
+if not secrets_backend_cls:
+return None
+print("secrets_backend_cls", secrets_backend_cls)
+secrets_backend = import_string(secrets_backend_cls)
+if secrets_backend:
+try:
+alternative_secrets_config_dict = json.loads(
+super().get('secrets', 'backend_kwargs', fallback='{}')
+)
+except JSONDecodeError:
Review comment:
Done in
https://github.com/apache/airflow/pull/9645/commits/a3ceb574fcef9c6cd6baea0bbcc58d08d2becd9e
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #9645: Get Airflow configs with sensitive data from Secret Backends
ashb commented on a change in pull request #9645:
URL: https://github.com/apache/airflow/pull/9645#discussion_r449639584
##
File path: airflow/configuration.py
##
@@ -267,17 +272,49 @@ def _get_env_var_option(self, section, key):
env_var_cmd = env_var + '_CMD'
if env_var_cmd in os.environ:
# if this is a valid command key...
-if (section, key) in self.as_command_stdout:
+if (section, key) in self.configs_with_secret:
return run_command(os.environ[env_var_cmd])
def _get_cmd_option(self, section, key):
fallback_key = key + '_cmd'
# if this is a valid command key...
-if (section, key) in self.as_command_stdout:
+if (section, key) in self.configs_with_secret:
if super().has_option(section, fallback_key):
command = super().get(section, fallback_key)
return run_command(command)
+@cached_property
+def _secrets_backend_client(self):
+if super().has_option('secrets', 'backend') is False:
+return None
+
+secrets_backend_cls = super().get('secrets', 'backend')
+if not secrets_backend_cls:
+return None
+print("secrets_backend_cls", secrets_backend_cls)
+secrets_backend = import_string(secrets_backend_cls)
+if secrets_backend:
+try:
+alternative_secrets_config_dict = json.loads(
+super().get('secrets', 'backend_kwargs', fallback='{}')
+)
+except JSONDecodeError:
Review comment:
```suggestion
except json.JSONDecodeError:
```
Then we don't have to have two import statements.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] kaxil opened a new pull request #9645: Get Airflow configs with sensitive data from Secret Backends
kaxil opened a new pull request #9645:
URL: https://github.com/apache/airflow/pull/9645
We should be able to get the followings config containing passwords and
other sensitive data from Secret Backends:
```
('core', 'sql_alchemy_conn'),
('core', 'fernet_key'),
('celery', 'broker_url'),
('celery', 'flower_basic_auth'),
('celery', 'result_backend'),
('atlas', 'password'),
('smtp', 'smtp_password'),
('ldap', 'bind_password'),
('kubernetes', 'git_password'),
```
---
Make sure to mark the boxes below before creating PR: [x]
- [x] Description above provides context of the change
- [x] Unit tests coverage for changes (not needed for documentation changes)
- [x] Target Github ISSUE in description if exists
- [x] Commits follow "[How to write a good git commit
message](http://chris.beams.io/posts/git-commit/)"
- [x] Relevant documentation is updated including usage instructions.
- [x] I will engage committers as explained in [Contribution Workflow
Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
---
In case of fundamental code change, Airflow Improvement Proposal
([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals))
is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party
License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in
[UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
Read the [Pull Request
Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)
for more information.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #9637: breeze Dockerfile.ci gcc ver
potiuk commented on pull request #9637: URL: https://github.com/apache/airflow/pull/9637#issuecomment-653598471 @xwydq - Maybe a better solution will be to do what I've done in Production dockerfile - to add build-essential instead of manually adding gcc ? i think we are not concerned about size of CI image too much but it might be better for any future needs. https://github.com/apache/airflow/blob/master/Dockerfile#L97 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #9531: Support .airflowignore for plugins
turbaszek commented on a change in pull request #9531:
URL: https://github.com/apache/airflow/pull/9531#discussion_r449633646
##
File path: tests/plugins/test_plugin_ignore.py
##
@@ -0,0 +1,96 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+import os
+import shutil
+import tempfile
+import unittest
+from unittest.mock import patch
+
+from airflow import settings # type: ignore
+from airflow.utils.file import find_path_from_directory # type: ignore
+
+
+class TestIgnorePluginFile(unittest.TestCase):
+"""
+Test that the .airflowignore work and whether the file is properly ignored.
+"""
+
+def setUp(self):
+"""
+Make tmp folder and files that should be ignored. And set base path.
+"""
+self.test_dir = tempfile.mkdtemp()
+self.test_file = os.path.join(self.test_dir, 'test_file.txt')
+self.plugin_folder_path = os.path.join(self.test_dir, 'test_ignore')
+os.mkdir(os.path.join(self.test_dir, "test_ignore"))
+with open(os.path.join(self.plugin_folder_path, "test_load.py"), "w")
as file:
+file.write("#Should not be ignored file")
+with open(os.path.join(self.plugin_folder_path, ".airflowignore"),
"w") as file:
+file.write("#ignore test\nnot\nsubdir2")
+os.mkdir(os.path.join(self.plugin_folder_path, "subdir1"))
+with open(os.path.join(self.plugin_folder_path,
"subdir1/.airflowignore"), "w") as file:
+file.write("#ignore test\nnone")
+with open(os.path.join(self.plugin_folder_path,
"subdir1/test_load_sub1.py"), "w") as file:
+file.write("#Should not be ignored file")
+with open(os.path.join(self.plugin_folder_path,
"test_notload_sub.py"), 'w') as file:
+file.write('raise Exception("This file should have been
ignored!")')
+with open(os.path.join(self.plugin_folder_path,
"subdir1/test_noneload_sub1.py"), 'w') as file:
+file.write('raise Exception("This file should have been
ignored!")')
+os.mkdir(os.path.join(self.plugin_folder_path, "subdir2"))
+with open(os.path.join(self.plugin_folder_path,
"subdir2/test_shouldignore.py"), 'w') as file:
+file.write('raise Exception("This file should have been
ignored!")')
+with open(os.path.join(self.plugin_folder_path,
"subdir2/test_shouldignore.py"), 'w') as file:
+file.write('raise Exception("This file should have been
ignored!")')
Review comment:
Is content of those files important? There's a lot of repeated code so I
would opt for some loop like:
```python
for file_path, content in files_content:
with open(file_path) as f:
f.wrtie(content)
```
Do you think it will make the code clearer?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] aneesh-joseph opened a new pull request #9644: add token_path param to vault client
aneesh-joseph opened a new pull request #9644: URL: https://github.com/apache/airflow/pull/9644 --- Make sure to mark the boxes below before creating PR: [x] - [ ] Description above provides context of the change - [ ] Unit tests coverage for changes (not needed for documentation changes) - [ ] Target Github ISSUE in description if exists - [ ] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)" - [ ] Relevant documentation is updated including usage instructions. - [ ] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example). --- In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed. In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md). Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information. Add `token_path` parameter to the vault client, hook and secret backend so that we can supply the file containing the token instead of explicitly providing the token value This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk closed issue #8416: Clustered Table is Not supported in BQ Operator Airflow 1.10.1
potiuk closed issue #8416: URL: https://github.com/apache/airflow/issues/8416 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk closed issue #9533: Upstream failed whereas Upstream is alright
potiuk closed issue #9533: URL: https://github.com/apache/airflow/issues/9533 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk closed issue #9324: How to create a role that can browse DAGs but not pause/unpause them?
potiuk closed issue #9324: URL: https://github.com/apache/airflow/issues/9324 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk closed issue #9086: Task Instance Slots Available' FAILED
potiuk closed issue #9086: URL: https://github.com/apache/airflow/issues/9086 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk closed issue #8716: Support on packaged dags
potiuk closed issue #8716: URL: https://github.com/apache/airflow/issues/8716 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk closed issue #9017: Error on running this docker on kubernetes
potiuk closed issue #9017: URL: https://github.com/apache/airflow/issues/9017 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk closed issue #7883: KubernetesPodOperator had an event of type Pending
potiuk closed issue #7883: URL: https://github.com/apache/airflow/issues/7883 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk closed issue #8656: How to make Airflow to get DAGs from S3 bucket if KubernetesExecutor is used?
potiuk closed issue #8656: URL: https://github.com/apache/airflow/issues/8656 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk closed issue #7955: Passing network and config/secret to DockerSwarmOperator
potiuk closed issue #7955: URL: https://github.com/apache/airflow/issues/7955 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk closed issue #8643: Can't setup enviroments for use secrets in docker-swarm
potiuk closed issue #8643: URL: https://github.com/apache/airflow/issues/8643 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk closed issue #8905: Logging with Custom Filenames
potiuk closed issue #8905: URL: https://github.com/apache/airflow/issues/8905 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk closed issue #9642: Wrong default mime_charset in Airflow 1.10.10
potiuk closed issue #9642: URL: https://github.com/apache/airflow/issues/9642 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk closed issue #9643: SSL connection has been closed unexpectedly
potiuk closed issue #9643: URL: https://github.com/apache/airflow/issues/9643 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk closed issue #9036: Task instance log_filepath doesn't include try_number
potiuk closed issue #9036: URL: https://github.com/apache/airflow/issues/9036 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk closed issue #9519: How to use private_key for SSH Operator
potiuk closed issue #9519: URL: https://github.com/apache/airflow/issues/9519 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk closed issue #8906: make success failed
potiuk closed issue #8906: URL: https://github.com/apache/airflow/issues/8906 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #9643: SSL connection has been closed unexpectedly
potiuk commented on issue #9643: URL: https://github.com/apache/airflow/issues/9643#issuecomment-653590257 Unfortunately this is not a generic problem - it looks like this is not a generic airlfow problem but rather your configuration problem so it falls under "support request/query" which should be discussed and raised in Slack, not as github 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] bhavaniravi opened a new issue #9643: SSL connection has been closed unexpectedly
bhavaniravi opened a new issue #9643:
URL: https://github.com/apache/airflow/issues/9643
**Apache Airflow version**:
1.10.11
**Kubernetes version (if you are using kubernetes)** (use `kubectl version`):
1.17.2
**Environment**:
- **Cloud provider or hardware configuration**: Self hosted kube cluster
- **OS** (e.g. from /etc/os-release): Debian GNU/Linux
- **Kernel** (e.g. `uname -a`): Linux
- **Install tools**:
- **Others**:
**What happened**:
Postgres SSL connection closed unexpectedly + Cannot write S3 logs
Both the errors happen together or separately
**What you expected to happen**:
1. Operational Errors should be graciously handled
2. S3 logs failure should retry on failure
**How to reproduce it**:
1. Create a task that connects to same postgres as airflow DB
2. Let airflow update it's metastore, update the DB with queries from
`PythonOperator`
3. Write logs to S3
**Anything else we need to know**:
3/5 - Pods has this error
1/5 - Pods has this error + S3 failure
Any relevant logs to include? Put them here inside a detail tag:
Log from worker pod
```
ERROR: airflow.jobs.local_task_job.LocalTaskJob LocalTaskJob heartbeat got
an exception
Traceback (most recent call last):
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/engine/base.py",
line 2345, in _wrap_pool_connect
return fn()
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/pool/base.py", line
364, in connect
return _ConnectionFairy._checkout(self)
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/pool/base.py", line
778, in _checkout
fairy = _ConnectionRecord.checkout(pool)
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/pool/base.py", line
495, in checkout
rec = pool._do_get()
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/pool/impl.py", line
239, in _do_get
return self._create_connection()
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/pool/base.py", line
309, in _create_connection
return _ConnectionRecord(self)
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/pool/base.py", line
440, in __init__
self.__connect(first_connect_check=True)
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/pool/base.py", line
661, in __connect
pool.logger.debug("Error on connect(): %s", e)
File
"/usr/local/lib/python3.6/site-packages/sqlalchemy/util/langhelpers.py", line
69, in __exit__
exc_value, with_traceback=exc_tb,
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/util/compat.py",
line 178, in raise_
raise exception
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/pool/base.py", line
656, in __connect
connection = pool._invoke_creator(self)
File
"/usr/local/lib/python3.6/site-packages/sqlalchemy/engine/strategies.py", line
114, in connect
return dialect.connect(*cargs, **cparams)
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/engine/default.py",
line 490, in connect
return self.dbapi.connect(*cargs, **cparams)
File "/usr/local/lib/python3.6/site-packages/psycopg2/__init__.py", line
127, in connect
conn = _connect(dsn, connection_factory=connection_factory, **kwasync)
psycopg2.OperationalError: SSL connection has been closed unexpectedly
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/usr/local/lib/python3.6/site-packages/airflow/jobs/base_job.py", line
199, in heartbeat
self.heartbeat_callback(session=session)
File "/usr/local/lib/python3.6/site-packages/airflow/utils/db.py", line 70,
in wrapper
return func(*args, **kwargs)
File
"/usr/local/lib/python3.6/site-packages/airflow/jobs/local_task_job.py", line
142, in heartbeat_callback
self.task_instance.refresh_from_db()
File "/usr/local/lib/python3.6/site-packages/airflow/utils/db.py", line 74,
in wrapper
return func(*args, **kwargs)
File
"/usr/local/lib/python3.6/site-packages/airflow/models/taskinstance.py", line
474, in refresh_from_db
ti = qry.first()
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/orm/query.py", line
3375, in first
ret = list(self[0:1])
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/orm/query.py", line
3149, in __getitem__
return list(res)
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/orm/query.py", line
3481, in __iter__
return self._execute_and_instances(context)
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/orm/query.py", line
3503, in _execute_and_instances
querycontext, self._connection_from_session, close_with_result=True
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/orm/query.py", line
3518, in _get_bind_args
mapper=self._bind_mapper(), clause=querycontext.statement, **kw
File "/usr/local/lib/python3.6/site-packages/sqlalchemy/orm/query.py", line
3496, in
[GitHub] [airflow] j-y-matsubara commented on a change in pull request #9531: Support .airflowignore for plugins
j-y-matsubara commented on a change in pull request #9531: URL: https://github.com/apache/airflow/pull/9531#discussion_r449615325 ## File path: tests/plugins/test_ignore/subdir1/test_load_sub1.py ## @@ -0,0 +1,35 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Import module""" +from airflow.models.baseoperator import BaseOperator # type: ignore +from airflow.utils.decorators import apply_defaults # type: ignore + + +class Sub1TestLoadOperator(BaseOperator): +""" +Test load operator +""" +@apply_defaults +def __init__( +self, +*args, +**kwargs): +super(Sub1TestLoadOperator, self).__init__(*args, **kwargs) + +def execute(self, context): +pass Review comment: These files were used to files that should not be ignored. ( `self.assertEqual(detected_files, should_not_ignore_files)` Line 87 (now 95)of the test_plugin_ignore.py. ) But I fixed these files and ".airflowignore" files to be generated by test_plugin_ignore.py, and delete pull files! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] j-y-matsubara commented on a change in pull request #9531: Support .airflowignore for plugins
j-y-matsubara commented on a change in pull request #9531: URL: https://github.com/apache/airflow/pull/9531#discussion_r449615325 ## File path: tests/plugins/test_ignore/subdir1/test_load_sub1.py ## @@ -0,0 +1,35 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Import module""" +from airflow.models.baseoperator import BaseOperator # type: ignore +from airflow.utils.decorators import apply_defaults # type: ignore + + +class Sub1TestLoadOperator(BaseOperator): +""" +Test load operator +""" +@apply_defaults +def __init__( +self, +*args, +**kwargs): +super(Sub1TestLoadOperator, self).__init__(*args, **kwargs) + +def execute(self, context): +pass Review comment: These files were used to files that should not be ignored. ( `self.assertEqual(detected_files, should_not_ignore_files)` Line 87 (now 95)of the test_plugin_ignore.py. ) But I fixed these files to be generated by test_plugin_ignore.py, and delete these files! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] j-y-matsubara commented on a change in pull request #9531: Support .airflowignore for plugins
j-y-matsubara commented on a change in pull request #9531: URL: https://github.com/apache/airflow/pull/9531#discussion_r449615325 ## File path: tests/plugins/test_ignore/subdir1/test_load_sub1.py ## @@ -0,0 +1,35 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Import module""" +from airflow.models.baseoperator import BaseOperator # type: ignore +from airflow.utils.decorators import apply_defaults # type: ignore + + +class Sub1TestLoadOperator(BaseOperator): +""" +Test load operator +""" +@apply_defaults +def __init__( +self, +*args, +**kwargs): +super(Sub1TestLoadOperator, self).__init__(*args, **kwargs) + +def execute(self, context): +pass Review comment: These files were used to files that should not be ignored. ( `self.assertEqual(detected_files, should_not_ignore_files)` Line 87 (now 95)of the test_plugin_ignore.py. ) But I fixed these files to be generated by test_plugin_ignore.py, and delete pull files! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on issue #9642: Wrong default mime_charset in Airflow 1.10.10
boring-cyborg[bot] commented on issue #9642: URL: https://github.com/apache/airflow/issues/9642#issuecomment-653578538 Thanks for opening your first issue here! Be sure to follow the issue template! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] j-y-matsubara commented on a change in pull request #9531: Support .airflowignore for plugins
j-y-matsubara commented on a change in pull request #9531: URL: https://github.com/apache/airflow/pull/9531#discussion_r449615325 ## File path: tests/plugins/test_ignore/subdir1/test_load_sub1.py ## @@ -0,0 +1,35 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Import module""" +from airflow.models.baseoperator import BaseOperator # type: ignore +from airflow.utils.decorators import apply_defaults # type: ignore + + +class Sub1TestLoadOperator(BaseOperator): +""" +Test load operator +""" +@apply_defaults +def __init__( +self, +*args, +**kwargs): +super(Sub1TestLoadOperator, self).__init__(*args, **kwargs) + +def execute(self, context): +pass Review comment: These files were used to files that should not be ignored. ( `self.assertEqual(detected_files, should_not_ignore_files)` Line 87 (now 95)of the test_plugin_ignore.py. ) But I fixed these files to be generated by test_plugin_ignore.py. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] Libum opened a new issue #9642: Wrong default mime_charset in Airflow 1.10.10
Libum opened a new issue #9642: URL: https://github.com/apache/airflow/issues/9642 Hello, I've encountered issue with default `mime_charset` in send_email utility in Airflow 1.10.10: https://github.com/apache/airflow/blob/317b0412383ccda571fbef568c9eabd70ab8e666/airflow/utils/email.py#L67 It is set to 'us-ascii', but I would expect 'utf-8' based on UPDATING.md and current state of master branch: https://github.com/apache/airflow/blob/v1-10-stable/UPDATING.md#setting-utf-8-as-default-mime_charset-in-email-utils https://github.com/apache/airflow/blob/fddc5721c9b5015cd600eec85496c7fc4bd513a7/airflow/utils/email.py#L52 As result emails are not sent, if they contain non-ascii chars. Best regards, Michal This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] j-y-matsubara commented on a change in pull request #9531: Support .airflowignore for plugins
j-y-matsubara commented on a change in pull request #9531:
URL: https://github.com/apache/airflow/pull/9531#discussion_r449615262
##
File path: tests/plugins/test_plugin_ignore.py
##
@@ -0,0 +1,89 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+import os
+import shutil
+import tempfile
+import unittest
+from unittest.mock import patch
+
+from airflow import settings # type: ignore
+from airflow.utils.file import find_path_from_directory # type: ignore
+
+
+class TestIgnorePluginFile(unittest.TestCase):
+"""
+Test that the .airflowignore work and whether the file is properly ignored.
+"""
+
+def setUp(self):
+"""
+Make tmp folder and files that should be ignored. And set base path.
+"""
+self.test_dir = tempfile.mkdtemp()
+self.test_file = os.path.join(self.test_dir, 'test_file.txt')
+self.plugin_folder_path = os.path.join(self.test_dir, 'test_ignore')
+shutil.copytree(os.path.join(settings.PLUGINS_FOLDER, 'test_ignore'),
self.plugin_folder_path)
+file = open(os.path.join(self.plugin_folder_path,
"test_notload_sub.py"), 'w')
+file.write('raise Exception("This file should have been ignored!")')
+file.close()
+file = open(os.path.join(self.plugin_folder_path,
"subdir1/test_noneload_sub1.py"), 'w')
+file.write('raise Exception("This file should have been ignored!")')
+file.close()
+os.mkdir(os.path.join(self.plugin_folder_path, "subdir2"))
+file = open(os.path.join(self.plugin_folder_path,
"subdir2/test_shouldignore.py"), 'w')
+file.write('raise Exception("This file should have been ignored!")')
+file.close()
Review comment:
Of course!
I fixed.
##
File path: tests/plugins/test_plugin_ignore.py
##
@@ -0,0 +1,89 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+import os
+import shutil
+import tempfile
+import unittest
+from unittest.mock import patch
+
+from airflow import settings # type: ignore
+from airflow.utils.file import find_path_from_directory # type: ignore
+
+
+class TestIgnorePluginFile(unittest.TestCase):
+"""
+Test that the .airflowignore work and whether the file is properly ignored.
+"""
+
+def setUp(self):
+"""
+Make tmp folder and files that should be ignored. And set base path.
+"""
+self.test_dir = tempfile.mkdtemp()
+self.test_file = os.path.join(self.test_dir, 'test_file.txt')
+self.plugin_folder_path = os.path.join(self.test_dir, 'test_ignore')
+shutil.copytree(os.path.join(settings.PLUGINS_FOLDER, 'test_ignore'),
self.plugin_folder_path)
+file = open(os.path.join(self.plugin_folder_path,
"test_notload_sub.py"), 'w')
+file.write('raise Exception("This file should have been ignored!")')
+file.close()
+file = open(os.path.join(self.plugin_folder_path,
"subdir1/test_noneload_sub1.py"), 'w')
+file.write('raise Exception("This file should have been ignored!")')
+file.close()
+os.mkdir(os.path.join(self.plugin_folder_path, "subdir2"))
+file = open(os.path.join(self.plugin_folder_path,
"subdir2/test_shouldignore.py"), 'w')
+file.write('raise Exception("This file should have been ignored!")')
+file.close()
+self.mock_plugins_folder = patch.object(
+settings, 'PLUGINS_FOLDER', return_value=self.plugin_folder_path
+
[GitHub] [airflow] j-y-matsubara commented on a change in pull request #9531: Support .airflowignore for plugins
j-y-matsubara commented on a change in pull request #9531: URL: https://github.com/apache/airflow/pull/9531#discussion_r449615325 ## File path: tests/plugins/test_ignore/subdir1/test_load_sub1.py ## @@ -0,0 +1,35 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Import module""" +from airflow.models.baseoperator import BaseOperator # type: ignore +from airflow.utils.decorators import apply_defaults # type: ignore + + +class Sub1TestLoadOperator(BaseOperator): +""" +Test load operator +""" +@apply_defaults +def __init__( +self, +*args, +**kwargs): +super(Sub1TestLoadOperator, self).__init__(*args, **kwargs) + +def execute(self, context): +pass Review comment: These files were used to files that should not be ignored. ( `self.assertEqual(detected_files, should_not_ignore_files)` Line 87 of the test_plugin_ignore.py. ) But I fixed these files to be generated by test_plugin_ignore.py. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] elwinarens commented on a change in pull request #9639: Add secrets backend for microsoft azure key vault
elwinarens commented on a change in pull request #9639:
URL: https://github.com/apache/airflow/pull/9639#discussion_r449562999
##
File path: airflow/providers/microsoft/azure/secrets/azure_key_vault.py
##
@@ -0,0 +1,107 @@
+"""
+This module contains a secrets backend for Azure Key Vault.
+"""
+from typing import Optional
+
+from azure.identity import DefaultAzureCredential
+from azure.keyvault.secrets import SecretClient
+from azure.core.exceptions import ResourceNotFoundError
+from cached_property import cached_property
+
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class AzureKeyVaultBackend(BaseSecretsBackend, LoggingMixin):
+"""
+Retrieves Airflow Connections or Variables from Azure Key Vault secrets.
+
+The Azure Key Vault can be configred as a secrets backend in the
``airflow.cfg``:
+
+.. code-block:: ini
+
+[secrets]
+backend =
airflow.providers.microsoft.azure.secrets.azure_key_vault.AzureKeyVaultBackend
+backend_kwargs = {"vault_url": ""}
+
+For example, if the secrets prefix is
``airflow-connections-smtp-default``, this would be accessible
+if you provide ``{"connections_prefix": "airflow-connections"}`` and
request conn_id ``smtp-default``.
+And if variables prefix is ``airflow-variables-hello``, this would be
accessible
+if you provide ``{"variables_prefix": "airflow-variables"}`` and request
variable key ``hello``.
+
+:param vault_url: The URL of an Azure Key Vault to use
+:type vault_url: str
+:param connections_prefix: Specifies the prefix of the secret to read to
get Connections
+:type connections_prefix: str
+:param variables_prefix: Specifies the prefix of the secret to read to get
Variables
+:type variables_prefix: str
+:param sep: separator used to concatenate secret_prefix and secret_id.
Default: "-"
+:type sep: str
+"""
+
+def __init__(self, vault_url: str = None, connections_prefix: str =
'airflow-connections',
+ variables_prefix: str = 'airflow-variables', sep: str = '-',
**kwargs):
+super().__init__(**kwargs)
+self.connections_prefix = connections_prefix.rstrip(sep)
+self.variables_prefix = variables_prefix.rstrip(sep)
+self.vault_url = vault_url
+self.sep = sep
+self.kwargs = kwargs
+
+@cached_property
+def client(self):
+"""
+Create a Azure Key Vault client.
+"""
+credential = DefaultAzureCredential()
Review comment:
More details
https://github.com/Azure/azure-sdk-for-python/tree/master/sdk/identity/azure-identity#defaultazurecredential
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on issue #9583: Attach roles to AD groups - Azure OAuth
mik-laj commented on issue #9583: URL: https://github.com/apache/airflow/issues/9583#issuecomment-653522828 Is this helpful for you? I don't use Azure OAuth, so I'm not sure this change applies here. https://github.com/dpgaspar/Flask-AppBuilder/pull/1410 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] elwinarens commented on a change in pull request #9639: Add secrets backend for microsoft azure key vault
elwinarens commented on a change in pull request #9639:
URL: https://github.com/apache/airflow/pull/9639#discussion_r449549782
##
File path: airflow/providers/microsoft/azure/secrets/azure_key_vault.py
##
@@ -0,0 +1,107 @@
+"""
+This module contains a secrets backend for Azure Key Vault.
+"""
+from typing import Optional
+
+from azure.identity import DefaultAzureCredential
+from azure.keyvault.secrets import SecretClient
+from azure.core.exceptions import ResourceNotFoundError
+from cached_property import cached_property
+
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class AzureKeyVaultBackend(BaseSecretsBackend, LoggingMixin):
+"""
+Retrieves Airflow Connections or Variables from Azure Key Vault secrets.
+
+The Azure Key Vault can be configred as a secrets backend in the
``airflow.cfg``:
+
+.. code-block:: ini
+
+[secrets]
+backend =
airflow.providers.microsoft.azure.secrets.azure_key_vault.AzureKeyVaultBackend
+backend_kwargs = {"vault_url": ""}
+
+For example, if the secrets prefix is
``airflow-connections-smtp-default``, this would be accessible
+if you provide ``{"connections_prefix": "airflow-connections"}`` and
request conn_id ``smtp-default``.
+And if variables prefix is ``airflow-variables-hello``, this would be
accessible
+if you provide ``{"variables_prefix": "airflow-variables"}`` and request
variable key ``hello``.
+
+:param vault_url: The URL of an Azure Key Vault to use
+:type vault_url: str
+:param connections_prefix: Specifies the prefix of the secret to read to
get Connections
+:type connections_prefix: str
+:param variables_prefix: Specifies the prefix of the secret to read to get
Variables
+:type variables_prefix: str
+:param sep: separator used to concatenate secret_prefix and secret_id.
Default: "-"
+:type sep: str
+"""
+
+def __init__(self, vault_url: str = None, connections_prefix: str =
'airflow-connections',
+ variables_prefix: str = 'airflow-variables', sep: str = '-',
**kwargs):
+super().__init__(**kwargs)
+self.connections_prefix = connections_prefix.rstrip(sep)
+self.variables_prefix = variables_prefix.rstrip(sep)
+self.vault_url = vault_url
+self.sep = sep
+self.kwargs = kwargs
+
+@cached_property
+def client(self):
+"""
+Create a Azure Key Vault client.
+"""
+credential = DefaultAzureCredential()
Review comment:
Would you mind elaborating?
##
File path: airflow/providers/microsoft/azure/secrets/azure_key_vault.py
##
@@ -0,0 +1,107 @@
+"""
+This module contains a secrets backend for Azure Key Vault.
+"""
+from typing import Optional
+
+from azure.identity import DefaultAzureCredential
+from azure.keyvault.secrets import SecretClient
+from azure.core.exceptions import ResourceNotFoundError
+from cached_property import cached_property
+
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class AzureKeyVaultBackend(BaseSecretsBackend, LoggingMixin):
+"""
+Retrieves Airflow Connections or Variables from Azure Key Vault secrets.
+
+The Azure Key Vault can be configred as a secrets backend in the
``airflow.cfg``:
+
+.. code-block:: ini
+
+[secrets]
+backend =
airflow.providers.microsoft.azure.secrets.azure_key_vault.AzureKeyVaultBackend
+backend_kwargs = {"vault_url": ""}
+
+For example, if the secrets prefix is
``airflow-connections-smtp-default``, this would be accessible
+if you provide ``{"connections_prefix": "airflow-connections"}`` and
request conn_id ``smtp-default``.
+And if variables prefix is ``airflow-variables-hello``, this would be
accessible
+if you provide ``{"variables_prefix": "airflow-variables"}`` and request
variable key ``hello``.
+
+:param vault_url: The URL of an Azure Key Vault to use
+:type vault_url: str
+:param connections_prefix: Specifies the prefix of the secret to read to
get Connections
+:type connections_prefix: str
+:param variables_prefix: Specifies the prefix of the secret to read to get
Variables
+:type variables_prefix: str
+:param sep: separator used to concatenate secret_prefix and secret_id.
Default: "-"
+:type sep: str
+"""
+
+def __init__(self, vault_url: str = None, connections_prefix: str =
'airflow-connections',
+ variables_prefix: str = 'airflow-variables', sep: str = '-',
**kwargs):
+super().__init__(**kwargs)
+self.connections_prefix = connections_prefix.rstrip(sep)
+self.variables_prefix = variables_prefix.rstrip(sep)
+self.vault_url = vault_url
+self.sep = sep
+self.kwargs = kwargs
+
+@cached_property
+def client(self):
+"""
+Create a Azure Key Vault client.
+
[GitHub] [airflow] elwinarens commented on a change in pull request #9639: Add secrets backend for microsoft azure key vault
elwinarens commented on a change in pull request #9639: URL: https://github.com/apache/airflow/pull/9639#discussion_r449549570 ## File path: tests/providers/microsoft/azure/secrets/test_azure_key_vault_backend.py ## @@ -0,0 +1,92 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from unittest import TestCase, mock + +from moto import mock_secretsmanager Review comment: Nice catch, copy pasta from aws. Will 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #9635: fix : _run_task_by_executor pickle_id is None
kaxil commented on a change in pull request #9635: URL: https://github.com/apache/airflow/pull/9635#discussion_r449548340 ## File path: airflow/cli/commands/task_command.py ## @@ -75,6 +75,7 @@ def _run_task_by_executor(args, dag, ti): with create_session() as session: Review comment: The create_session context manager commits the session before exiting the context ``` @contextlib.contextmanager def create_session(): """ Contextmanager that will create and teardown a session. """ session = settings.Session() try: yield session session.commit() except Exception: session.rollback() raise finally: session.close() ``` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] haidaraM commented on pull request #7407: [AIRFLOW-6786] Add KafkaConsumerHook, KafkaProduerHook and KafkaSensor
haidaraM commented on pull request #7407: URL: https://github.com/apache/airflow/pull/7407#issuecomment-653510340 Any news on this PR ? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] rafaelpierre commented on issue #9583: Attach roles to AD groups - Azure OAuth
rafaelpierre commented on issue #9583: URL: https://github.com/apache/airflow/issues/9583#issuecomment-653499207 @sk2991 +1 > @sk2991 Would you mind sharing your Azure OAuth implementation? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] ashb opened a new issue #9641: Double check UPDATING.md for "2.0" doesn't include things already released
ashb opened a new issue #9641: URL: https://github.com/apache/airflow/issues/9641 We need to double check the UDPDATING.md has been correctly updated on mainline to correctly reflect features that have already been included in 1.10.x releases Specifically that anything included in earlier releases has been removed from the section for 2.0 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] ashb opened a new issue #9640: Ensure Airflow 1.10.11 DB can be updated to master
ashb opened a new issue #9640: URL: https://github.com/apache/airflow/issues/9640 To ensure our users can upgrade from Airflow 1.10.11 to 2.0 (when it comes) we need to double check that the Alembic migration "history" is correct. This might involve either creating a merge point, or updating some of the migrations on master branch to reflect the correct history/ancestors for what has already been included in releases. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] nathadfield commented on pull request #8745: Install python-ldap library for FAB LDAP authorization with Python3
nathadfield commented on pull request #8745: URL: https://github.com/apache/airflow/pull/8745#issuecomment-653489782 This is definitely important given that the non FAB UI will ultimately be going away. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #9639: Add secrets backend for microsoft azure key vault
kaxil commented on a change in pull request #9639:
URL: https://github.com/apache/airflow/pull/9639#discussion_r449522855
##
File path:
tests/providers/microsoft/azure/secrets/test_azure_key_vault_backend.py
##
@@ -0,0 +1,92 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from unittest import TestCase, mock
+
+from moto import mock_secretsmanager
+
+from airflow.providers.microsoft.azure.secrets.azure_key_vault import
AzureKeyVaultBackend
+
+
+class TestAzureKeyVaultBackend(TestCase):
+@mock.patch("airflow.providers.microsoft.azure.secrets.azure_key_vault"
+"AzureKeyVaultBackend.get_conn_uri")
+def test_get_connections(self, mock_get_uri):
+mock_get_uri.return_value = "scheme://user:pass@host:100"
+conn_list = AzureKeyVaultBackend().get_connections("fake_conn")
+conn = conn_list[0]
+assert conn.host == 'host'
+
+@mock_secretsmanager
+def test_get_conn_uri(self):
+param = {
+'SecretId': 'airflow-connections-test_postgres',
+'SecretString': 'postgresql://airflow:airflow@host:5432/airflow'
+}
+
+backend = AzureKeyVaultBackend()
+backend.client.put_secret_value(**param)
+
+returned_uri = backend.get_conn_uri(conn_id="test_postgres")
+self.assertEqual('postgresql://airflow:airflow@host:5432/airflow',
returned_uri)
+
+@mock_secretsmanager
Review comment:
and here and everywhere in the file?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #9639: Add secrets backend for microsoft azure key vault
kaxil commented on a change in pull request #9639: URL: https://github.com/apache/airflow/pull/9639#discussion_r449522757 ## File path: tests/providers/microsoft/azure/secrets/test_azure_key_vault_backend.py ## @@ -0,0 +1,92 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from unittest import TestCase, mock + +from moto import mock_secretsmanager Review comment: Why do we use `moto` here? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #9639: Add secrets backend for microsoft azure key vault
kaxil commented on a change in pull request #9639:
URL: https://github.com/apache/airflow/pull/9639#discussion_r449521224
##
File path: airflow/providers/microsoft/azure/secrets/azure_key_vault.py
##
@@ -0,0 +1,107 @@
+"""
+This module contains a secrets backend for Azure Key Vault.
+"""
+from typing import Optional
+
+from azure.identity import DefaultAzureCredential
+from azure.keyvault.secrets import SecretClient
+from azure.core.exceptions import ResourceNotFoundError
+from cached_property import cached_property
+
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class AzureKeyVaultBackend(BaseSecretsBackend, LoggingMixin):
+"""
+Retrieves Airflow Connections or Variables from Azure Key Vault secrets.
+
+The Azure Key Vault can be configred as a secrets backend in the
``airflow.cfg``:
+
+.. code-block:: ini
+
+[secrets]
+backend =
airflow.providers.microsoft.azure.secrets.azure_key_vault.AzureKeyVaultBackend
+backend_kwargs = {"vault_url": ""}
+
+For example, if the secrets prefix is
``airflow-connections-smtp-default``, this would be accessible
+if you provide ``{"connections_prefix": "airflow-connections"}`` and
request conn_id ``smtp-default``.
+And if variables prefix is ``airflow-variables-hello``, this would be
accessible
+if you provide ``{"variables_prefix": "airflow-variables"}`` and request
variable key ``hello``.
+
+:param vault_url: The URL of an Azure Key Vault to use
+:type vault_url: str
+:param connections_prefix: Specifies the prefix of the secret to read to
get Connections
+:type connections_prefix: str
+:param variables_prefix: Specifies the prefix of the secret to read to get
Variables
+:type variables_prefix: str
+:param sep: separator used to concatenate secret_prefix and secret_id.
Default: "-"
+:type sep: str
+"""
+
+def __init__(self, vault_url: str = None, connections_prefix: str =
'airflow-connections',
+ variables_prefix: str = 'airflow-variables', sep: str = '-',
**kwargs):
+super().__init__(**kwargs)
+self.connections_prefix = connections_prefix.rstrip(sep)
+self.variables_prefix = variables_prefix.rstrip(sep)
+self.vault_url = vault_url
+self.sep = sep
+self.kwargs = kwargs
+
+@cached_property
+def client(self):
+"""
+Create a Azure Key Vault client.
+"""
+credential = DefaultAzureCredential()
Review comment:
What is the limitation of using `DefaultAzureCredential`
https://docs.microsoft.com/en-us/azure/developer/python/azure-sdk-authenticate?tabs=cmd
##
File path: airflow/providers/microsoft/azure/secrets/azure_key_vault.py
##
@@ -0,0 +1,107 @@
+"""
+This module contains a secrets backend for Azure Key Vault.
+"""
+from typing import Optional
+
+from azure.identity import DefaultAzureCredential
+from azure.keyvault.secrets import SecretClient
+from azure.core.exceptions import ResourceNotFoundError
+from cached_property import cached_property
+
+from airflow.secrets import BaseSecretsBackend
+from airflow.utils.log.logging_mixin import LoggingMixin
+
+
+class AzureKeyVaultBackend(BaseSecretsBackend, LoggingMixin):
+"""
+Retrieves Airflow Connections or Variables from Azure Key Vault secrets.
+
+The Azure Key Vault can be configred as a secrets backend in the
``airflow.cfg``:
+
+.. code-block:: ini
+
+[secrets]
+backend =
airflow.providers.microsoft.azure.secrets.azure_key_vault.AzureKeyVaultBackend
+backend_kwargs = {"vault_url": ""}
+
+For example, if the secrets prefix is
``airflow-connections-smtp-default``, this would be accessible
+if you provide ``{"connections_prefix": "airflow-connections"}`` and
request conn_id ``smtp-default``.
+And if variables prefix is ``airflow-variables-hello``, this would be
accessible
+if you provide ``{"variables_prefix": "airflow-variables"}`` and request
variable key ``hello``.
+
+:param vault_url: The URL of an Azure Key Vault to use
+:type vault_url: str
+:param connections_prefix: Specifies the prefix of the secret to read to
get Connections
+:type connections_prefix: str
+:param variables_prefix: Specifies the prefix of the secret to read to get
Variables
+:type variables_prefix: str
+:param sep: separator used to concatenate secret_prefix and secret_id.
Default: "-"
+:type sep: str
+"""
+
+def __init__(self, vault_url: str = None, connections_prefix: str =
'airflow-connections',
+ variables_prefix: str = 'airflow-variables', sep: str = '-',
**kwargs):
+super().__init__(**kwargs)
Review comment:
```suggestion
super().__init__()
```
This is an automated message from the Apache Git Service.
To
[GitHub] [airflow] turbaszek commented on a change in pull request #9631: Add function to get current context
turbaszek commented on a change in pull request #9631: URL: https://github.com/apache/airflow/pull/9631#discussion_r449521543 ## File path: airflow/task/context/current.py ## @@ -0,0 +1,69 @@ +# Licensed to the Apache Software Foundation (ASF) under one Review comment: @mik-laj what do you think about this localization? I'm wondering if it would be better to move this to `airflow/utils` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] turbaszek commented on a change in pull request #9631: Add function to get current context
turbaszek commented on a change in pull request #9631: URL: https://github.com/apache/airflow/pull/9631#discussion_r449521128 ## File path: airflow/task/context/current.py ## @@ -0,0 +1,69 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import contextlib +import logging +from typing import Any, Dict + +from airflow.exceptions import AirflowException + +_CURRENT_CONTEXT = [] +log = logging.getLogger(__name__) + + +@contextlib.contextmanager +def set_current_context(context: Dict[str, Any]): +""" +Sets the current execution context to the provided context object. +This method should be called once per Task execution, before calling operator.execute +""" +_CURRENT_CONTEXT.append(context) +try: +yield context +finally: +expected_state = _CURRENT_CONTEXT.pop() +if expected_state != context: +log.warning( +"Current context is not equal to the state at context stack. Expected=%s, got=%s", +context, +expected_state, +) + + +def get_current_context() -> Dict[str, Any]: +""" +Obtain the execution context for the currently executing operator without +altering user method's signature. +This is the simplest method of retrieving the execution context dictionary. +** Old style: +def my_task(**context): +ti = context["ti"] +** New style: +from airflow.task.context import get_current_context +def my_task(): +context = get_current_context() +ti = context["ti"] Review comment: Probably it's good idea to extend `@task` docs with this This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
