[airflow] branch main updated (dbeec89 -> 70bf1b1)
This is an automated email from the ASF dual-hosted git repository. ash pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/airflow.git. from dbeec89 Refactor: `SKIPPED` shouldn't be logged again as `SUCCESS`. (#14822) add 70bf1b1 Fix normalize-url vulnerability (#16375) No new revisions were added by this update. Summary of changes: airflow/www/package.json |4 +- airflow/www/yarn.lock| 1027 +- 2 files changed, 376 insertions(+), 655 deletions(-)
[airflow-client-python] branch master updated: Add NOTICE file (#25)
This is an automated email from the ASF dual-hosted git repository. kamilbregula pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/airflow-client-python.git The following commit(s) were added to refs/heads/master by this push: new bf439f3 Add NOTICE file (#25) bf439f3 is described below commit bf439f3f92b8248cb9f77adde250d4c4dbb860a9 Author: Kaxil Naik AuthorDate: Fri Jun 11 00:33:46 2021 +0100 Add NOTICE file (#25) * Add NOTICE file closes https://github.com/apache/airflow-client-python/issues/24 * Update setup.cfg --- NOTICE| 6 ++ setup.cfg | 3 +++ 2 files changed, 9 insertions(+) diff --git a/NOTICE b/NOTICE new file mode 100644 index 000..0ae7e64 --- /dev/null +++ b/NOTICE @@ -0,0 +1,6 @@ +Apache Airflow +Copyright 2016-2021 The Apache Software Foundation + +This product includes software developed at The Apache Software +Foundation (http://www.apache.org/). +=== diff --git a/setup.cfg b/setup.cfg index c6ebeea..84742b7 100644 --- a/setup.cfg +++ b/setup.cfg @@ -27,6 +27,9 @@ url = https://airflow.apache.org/ long_description = file: README.md long_description_content_type = text/markdown license = Apache License 2.0 +license_files = + LICENSE + NOTICE project_urls = Documentation=https://airflow.apache.org/docs/apache-airflow/stable/stable-rest-api-ref.html Bug Tracker=https://github.com/apache/airflow-client-python/issues
[airflow-client-python] branch kaxil-patch-1 updated (8ad3c66 -> 1d38b0b)
This is an automated email from the ASF dual-hosted git repository. kaxilnaik pushed a change to branch kaxil-patch-1 in repository https://gitbox.apache.org/repos/asf/airflow-client-python.git. from 8ad3c66 Add NOTICE file add 1d38b0b Update setup.cfg No new revisions were added by this update. Summary of changes: setup.cfg | 3 +++ 1 file changed, 3 insertions(+)
[GitHub] [airflow-client-python] kaxil opened a new pull request #25: Add NOTICE file
kaxil opened a new pull request #25: URL: https://github.com/apache/airflow-client-python/pull/25 closes https://github.com/apache/airflow-client-python/issues/24 -- This is an automated message from the 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-client-python] branch kaxil-patch-1 created (now 8ad3c66)
This is an automated email from the ASF dual-hosted git repository. kaxilnaik pushed a change to branch kaxil-patch-1 in repository https://gitbox.apache.org/repos/asf/airflow-client-python.git. at 8ad3c66 Add NOTICE file This branch includes the following new commits: new 8ad3c66 Add NOTICE file The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference.
[airflow-client-python] 01/01: Add NOTICE file
This is an automated email from the ASF dual-hosted git repository. kaxilnaik pushed a commit to branch kaxil-patch-1 in repository https://gitbox.apache.org/repos/asf/airflow-client-python.git commit 8ad3c66eba3bb6703ef590c06a8432537f61a739 Author: Kaxil Naik AuthorDate: Fri Jun 11 00:22:42 2021 +0100 Add NOTICE file closes https://github.com/apache/airflow-client-python/issues/24 --- NOTICE | 6 ++ 1 file changed, 6 insertions(+) diff --git a/NOTICE b/NOTICE new file mode 100644 index 000..0ae7e64 --- /dev/null +++ b/NOTICE @@ -0,0 +1,6 @@ +Apache Airflow +Copyright 2016-2021 The Apache Software Foundation + +This product includes software developed at The Apache Software +Foundation (http://www.apache.org/). +===
[GitHub] [airflow-client-python] kaxil opened a new issue #24: Add NOTICE file
kaxil opened a new issue #24: URL: https://github.com/apache/airflow-client-python/issues/24 Add NOTICE file similar to https://github.com/apache/airflow/blob/main/chart/NOTICE -- This is an automated message from the 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] github-actions[bot] commented on pull request #16327: Make job name check optional in SageMakerTrainingOperator
github-actions[bot] commented on pull request #16327: URL: https://github.com/apache/airflow/pull/16327#issuecomment-859147260 The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease. -- This is an automated message from the 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] SamWheating edited a comment on pull request #14306: Adding `only_active` parameter to /dags endpoint
SamWheating edited a comment on pull request #14306: URL: https://github.com/apache/airflow/pull/14306#issuecomment-859116694 Ah, looks like the original issue leading to this PR was fixed in https://github.com/apache/airflow/pull/16318, which prevents the API endpoint from ever returning inactive DAGs. I'll refactor this PR accordingly, but the changes introduced by this PR are now: - Expose an only_active parameter on the /dags endpoint (deafult to True) - Include `is_active` and `is_paused` in the DAG detail schema Update: I've updated + rebased this PR, should be good to go 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] SamWheating edited a comment on pull request #14306: Adding `only_active` parameter to /dags endpoint
SamWheating edited a comment on pull request #14306: URL: https://github.com/apache/airflow/pull/14306#issuecomment-859116694 Ah, looks like the original issue leading to this PR was fixed in https://github.com/apache/airflow/pull/16318, which prevents the API endpoint from ever returning inactive DAGs. I'll refactor this PR accordingly, but the changes introduced by this PR are now: - Expose an only_active parameter on the /dags endpoint (deafult to True) - Include `is_active` and `is_paused` in the DAG detail schema -- This is an automated message from the 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] SamWheating commented on pull request #14306: Adding `only_active` parameter to /dags endpoint
SamWheating commented on pull request #14306: URL: https://github.com/apache/airflow/pull/14306#issuecomment-859116694 Ah, looks like the original issue leading to this PR was fixed in https://github.com/apache/airflow/pull/16318, which prevents the API endpoint from ever returning inactive DAGs. I'll refactor this PR accordingly, but I believe the change now will only be to expose this functionality as optional rather than enforced one way or the other. -- This is an automated message from the 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 pull request #16358: Only create dagruns when max_active_runs is not reached
ephraimbuddy commented on pull request #16358: URL: https://github.com/apache/airflow/pull/16358#issuecomment-859116510 > I haven't looked into the details. But based on the idea I get from the PR subject, this may affect the DAGs whose 'catchup' is False It works fine in both I _think_ -- This is an automated message from the 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 closed pull request #14440: Bump openapi-spec-validator to 0.3
kaxil closed pull request #14440: URL: https://github.com/apache/airflow/pull/14440 -- This is an automated message from the 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 #16182: Do not queue tasks when the DAG file goes missing
kaxil commented on a change in pull request #16182: URL: https://github.com/apache/airflow/pull/16182#discussion_r649565887 ## File path: airflow/models/dagbag.py ## @@ -166,6 +166,36 @@ def dag_ids(self) -> List[str]: """ return list(self.dags.keys()) +@provide_session +def has_dag(self, dag_id: str, session: Session = None): +""" +Checks the "local" cache, if it exists, and is not older than the confiured cache time, return True +else check in the DB if the dag exists, return True, False otherwise +""" +if dag_id in self.dags: +min_serialized_dag_fetch_secs = timedelta(seconds=settings.MIN_SERIALIZED_DAG_FETCH_INTERVAL) +if dag_id in self.dags_last_fetched: + +return timezone.utcnow() < self.dags_last_fetched[dag_id] + min_serialized_dag_fetch_secs Review comment: Once https://github.com/apache/airflow/pull/16368 is merged, you can just call `get_dag(dag_id)`, if it returns `None`, you can return False -- This is an automated message from the 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] jedcunningham commented on a change in pull request #16368: Don't show stale Serialized DAGs if they are deleted in DB
jedcunningham commented on a change in pull request #16368:
URL: https://github.com/apache/airflow/pull/16368#discussion_r649565547
##
File path: airflow/models/dagbag.py
##
@@ -199,7 +201,13 @@ def get_dag(self, dag_id, session: Session = None):
dag_id=dag_id,
session=session,
)
-if sd_last_updated_datetime and sd_last_updated_datetime >
self.dags_last_fetched[dag_id]:
+if not sd_last_updated_datetime:
+self.log.warning("Serialized DAG does not exist for '%s'",
dag_id)
+del self.dags[dag_id]
+del self.dags_last_fetched[dag_id]
+del self.dags_hash[dag_id]
+return None
+elif sd_last_updated_datetime > self.dags_last_fetched[dag_id]:
self._add_dag_from_db(dag_id=dag_id, session=session)
Review comment:
```suggestion
if sd_last_updated_datetime > self.dags_last_fetched[dag_id]:
self._add_dag_from_db(dag_id=dag_id, session=session)
```
nit, since we short circuit
--
This is an automated message from the 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 #16182: Do not queue tasks when the DAG file goes missing
kaxil commented on a change in pull request #16182:
URL: https://github.com/apache/airflow/pull/16182#discussion_r649564351
##
File path: airflow/jobs/scheduler_job.py
##
@@ -1417,6 +1420,42 @@ def _clean_tis_without_dagrun(self, session):
raise
guard.commit()
+@provide_session
+def _missing_dag_file_cleanup(self, session: Session = None):
+"""Fails task instances and DagRuns of DAGs that no longer exist in
the dag folder/DB"""
+states_to_check = State.unfinished - frozenset([State.NONE,
State.SHUTDOWN])
+tis = session.query(TI).filter(TI.state.in_(states_to_check)).all()
+missing_dags = {}
+dag_runs = set()
+
+for ti in tis:
+if ti.dag_id in missing_dags:
+ti.set_state(State.FAILED, session=session)
+continue
+# Dag no longer in dagbag?
+if not self.dagbag.has_dag(ti.dag_id, session=session):
+ti.set_state(State.FAILED, session=session)
+dag_runs.add(ti.dag_run)
+missing_dags[ti.dag_id] = [ti]
+continue
+# Dag file no longer exists?
+if not os.path.exists(ti.dag_model.fileloc):
+ti.set_state(State.FAILED, session=session)
+dag_runs.add(ti.dag_run)
+missing_dags[ti.dag_id] = [ti]
+if missing_dags:
+self.log.warning(
+"The following Dags are missing, therefore the DAG's "
+"task instances have been failed: \t\n%s",
Review comment:
```suggestion
"task instances have been set to failed: \t\n%s",
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #16182: Do not queue tasks when the DAG file goes missing
kaxil commented on a change in pull request #16182:
URL: https://github.com/apache/airflow/pull/16182#discussion_r649563576
##
File path: airflow/jobs/scheduler_job.py
##
@@ -1417,6 +1420,42 @@ def _clean_tis_without_dagrun(self, session):
raise
guard.commit()
+@provide_session
+def _missing_dag_file_cleanup(self, session: Session = None):
+"""Fails task instances and DagRuns of DAGs that no longer exist in
the dag folder/DB"""
+states_to_check = State.unfinished - frozenset([State.NONE,
State.SHUTDOWN])
+tis = session.query(TI).filter(TI.state.in_(states_to_check)).all()
+missing_dags = {}
+dag_runs = set()
+
+for ti in tis:
+if ti.dag_id in missing_dags:
+ti.set_state(State.FAILED, session=session)
+continue
+# Dag no longer in dagbag?
+if not self.dagbag.has_dag(ti.dag_id, session=session):
+ti.set_state(State.FAILED, session=session)
+dag_runs.add(ti.dag_run)
+missing_dags[ti.dag_id] = [ti]
Review comment:
Do we need to store `ti` here?
`missing_dags` can be just a set
```python
missing_dags = set()
```
and then this line can be:
```suggestion
missing_dags.add(ti.dag_id)
```
--
This is an automated message from the 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 #16182: Do not queue tasks when the DAG file goes missing
kaxil commented on a change in pull request #16182:
URL: https://github.com/apache/airflow/pull/16182#discussion_r649563576
##
File path: airflow/jobs/scheduler_job.py
##
@@ -1417,6 +1420,42 @@ def _clean_tis_without_dagrun(self, session):
raise
guard.commit()
+@provide_session
+def _missing_dag_file_cleanup(self, session: Session = None):
+"""Fails task instances and DagRuns of DAGs that no longer exist in
the dag folder/DB"""
+states_to_check = State.unfinished - frozenset([State.NONE,
State.SHUTDOWN])
+tis = session.query(TI).filter(TI.state.in_(states_to_check)).all()
+missing_dags = {}
+dag_runs = set()
+
+for ti in tis:
+if ti.dag_id in missing_dags:
+ti.set_state(State.FAILED, session=session)
+continue
+# Dag no longer in dagbag?
+if not self.dagbag.has_dag(ti.dag_id, session=session):
+ti.set_state(State.FAILED, session=session)
+dag_runs.add(ti.dag_run)
+missing_dags[ti.dag_id] = [ti]
Review comment:
Does we need to store `ti` here?
`missing_dags` can be just a set
```python
missing_dags = set()
```
and then this line can be:
```suggestion
missing_dags.add(ti.dag_id)
```
--
This is an automated message from the 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 #16379: Airflow Stable REST API [GET api/v1/pools] issue
boring-cyborg[bot] commented on issue #16379: URL: https://github.com/apache/airflow/issues/16379#issuecomment-859110898 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] rbohlmann1 opened a new issue #16379: Airflow Stable REST API [GET api/v1/pools] issue
rbohlmann1 opened a new issue #16379: URL: https://github.com/apache/airflow/issues/16379 **Apache Airflow version**: v2.0.2 **Kubernetes version (if you are using kubernetes)** (use `kubectl version`): N/A **Environment**: AWS - **Cloud provider or hardware configuration**: AWS EC2 Instance - **OS** (e.g. from /etc/os-release): Ubuntu Server 20.04 LTS - **Kernel** (e.g. `uname -a`): Linux ip-172-31-23-31 5.4.0-1048-aws #50-Ubuntu SMP Mon May 3 21:44:17 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux - **Install tools**: - **Others**: Python version: 3.8.5 **What happened**: Using Airflow Stable REST API [GET api/v1/pools] results in Ooops! This only occurs when the pools have "Running Slots". If no tasks are running and the slots are zero, then it works just fine. Something bad has happened. Please consider letting us know by creating a bug report using GitHub. Python version: 3.8.5 Airflow version: 2.0.2 Node: ip-172-31-23-31.ec2.internal --- Traceback (most recent call last): File "/home/tool/gto_env/lib/python3.8/site-packages/flask/app.py", line 2447, in wsgi_app response = self.full_dispatch_request() File "/home/tool/gto_env/lib/python3.8/site-packages/flask/app.py", line 1952, in full_dispatch_request rv = self.handle_user_exception(e) File "/home/tool/gto_env/lib/python3.8/site-packages/flask/app.py", line 1821, in handle_user_exception reraise(exc_type, exc_value, tb) File "/home/tool/gto_env/lib/python3.8/site-packages/flask/_compat.py", line 39, in reraise raise value File "/home/tool/gto_env/lib/python3.8/site-packages/flask/app.py", line 1950, in full_dispatch_request rv = self.dispatch_request() File "/home/tool/gto_env/lib/python3.8/site-packages/flask/app.py", line 1936, in dispatch_request return self.view_functions[rule.endpoint](**req.view_args) File "/home/tool/gto_env/lib/python3.8/site-packages/connexion/decorators/decorator.py", line 48, in wrapper response = function(request) File "/home/tool/gto_env/lib/python3.8/site-packages/connexion/decorators/uri_parsing.py", line 144, in wrapper response = function(request) File "/home/tool/gto_env/lib/python3.8/site-packages/connexion/decorators/validation.py", line 384, in wrapper return function(request) File "/home/tool/gto_env/lib/python3.8/site-packages/connexion/decorators/response.py", line 104, in wrapper return _wrapper(request, response) File "/home/tool/gto_env/lib/python3.8/site-packages/connexion/decorators/response.py", line 89, in _wrapper self.operation.api.get_connexion_response(response, self.mimetype) File "/home/tool/gto_env/lib/python3.8/site-packages/connexion/apis/abstract.py", line 351, in get_connexion_response response = cls._response_from_handler(response, mimetype) File "/home/tool/gto_env/lib/python3.8/site-packages/connexion/apis/abstract.py", line 331, in _response_from_handler return cls._build_response(mimetype=mimetype, data=response, extra_context=extra_context) File "/home/tool/gto_env/lib/python3.8/site-packages/connexion/apis/flask_api.py", line 173, in _build_response data, status_code, serialized_mimetype = cls._prepare_body_and_status_code(data=data, mimetype=mimetype, status_code=status_code, extra_context=extra_context) File "/home/tool/gto_env/lib/python3.8/site-packages/connexion/apis/abstract.py", line 403, in _prepare_body_and_status_code body, mimetype = cls._serialize_data(data, mimetype) File "/home/tool/gto_env/lib/python3.8/site-packages/connexion/apis/flask_api.py", line 190, in _serialize_data body = cls.jsonifier.dumps(data) File "/home/tool/gto_env/lib/python3.8/site-packages/connexion/jsonifier.py", line 44, in dumps return self.json.dumps(data, **kwargs) + '\n' File "/home/tool/gto_env/lib/python3.8/site-packages/flask/json/__init__.py", line 211, in dumps rv = _json.dumps(obj, **kwargs) File "/usr/lib/python3.8/json/__init__.py", line 234, in dumps return cls( File "/usr/lib/python3.8/json/encoder.py", line 201, in encode chunks = list(chunks) File "/usr/lib/python3.8/json/encoder.py", line 431, in _iterencode yield from _iterencode_dict(o, _current_indent_level) File "/usr/lib/python3.8/json/encoder.py", line 405, in _iterencode_dict yield from chunks File "/usr/lib/python3.8/json/encoder.py", line 325, in _iterencode_list yield from chunks File "/usr/lib/python3.8/json/encoder.py", line 405, in _iterencode_dict yield from chunks File "/usr/lib/python3.8/json/encoder.py", line 438, in _iterencode o = _default(o) File "/home/tool/gto_env/lib/python3.8/site-packages/airflow/utils/json.py",
[GitHub] [airflow] ephraimbuddy commented on pull request #16358: Only create dagruns when max_active_runs is not reached
ephraimbuddy commented on pull request #16358: URL: https://github.com/apache/airflow/pull/16358#issuecomment-859110569 > Does this PR literally restrict from staggering new dagruns of there's already running one? I don't think I understand you correctly but if you mean creating more dagruns when the max_active_runs have been reached, then yes. You can't create more runs if the max_active_runs has been reached -- This is an automated message from the 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] scrdest commented on pull request #11652: Support for sorting DAGs in the web UI
scrdest commented on pull request #11652: URL: https://github.com/apache/airflow/pull/11652#issuecomment-859107901 > > > @scrdest can you fix these merge conflicts? If so we'll target 2.2 for this release. > > @ashb bumping since you requested changes I'm happy to in principle, but @uranusjr 's comment made me leave this on the back-burner because I thought it would wind up getting closed -- This is an automated message from the 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 #16301: Fix task retries when they receive sigkill and have retries. Properly handle sigterm too
ephraimbuddy commented on a change in pull request #16301:
URL: https://github.com/apache/airflow/pull/16301#discussion_r649558802
##
File path: airflow/jobs/local_task_job.py
##
@@ -80,14 +80,15 @@ def signal_handler(signum, frame):
self.log.error("Received SIGTERM. Terminating subprocesses")
self.on_kill()
self.task_instance.refresh_from_db()
-if self.task_instance.state not in State.finished:
-self.task_instance.set_state(State.FAILED)
-self.task_instance._run_finished_callback( # pylint:
disable=protected-access
-error="task received sigterm"
-)
+if self.task_instance.state == State.RUNNING:
+# This only happens when a pod is deleted
+# killing the task_runner by sending sigterm.
+# TI sigterm is handled at taskinstance.py
+self.handle_task_exit(128 + signum)
Review comment:
@ashb The test I had previously for sigterm was not hitting here. I'm
only able to hit here when I delete a running kubernetes pod. I wonder how we
can test this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #14306: Adding `only_active` parameter to /dags endpoint
kaxil commented on pull request #14306: URL: https://github.com/apache/airflow/pull/14306#issuecomment-859101221 @SamWheating can you rebase your PR once more and fix merge conflicts too please -- This is an automated message from the 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 #14822: `SKIPPED` shouldn't be logged again as `SUCCESS`.
kaxil commented on pull request #14822: URL: https://github.com/apache/airflow/pull/14822#issuecomment-859100198 Static failure is fixed in master already -- This is an automated message from the 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 main updated: Refactor: `SKIPPED` shouldn't be logged again as `SUCCESS`. (#14822)
This is an automated email from the ASF dual-hosted git repository.
kaxilnaik pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new dbeec89 Refactor: `SKIPPED` shouldn't be logged again as `SUCCESS`.
(#14822)
dbeec89 is described below
commit dbeec896fd752f266d1fd9950ba9220d415231b9
Author: suiting-young <80445042+suiting-yo...@users.noreply.github.com>
AuthorDate: Fri Jun 11 05:48:51 2021 +0800
Refactor: `SKIPPED` shouldn't be logged again as `SUCCESS`. (#14822)
* `SKIPPED` shouldn't be logged again as `SUCCESS`.
* `_safe_date` duplicates with `_date_or_empty`.
* Borrowed advantage from `_safe_date`.
---
airflow/models/taskinstance.py | 65 +-
1 file changed, 20 insertions(+), 45 deletions(-)
diff --git a/airflow/models/taskinstance.py b/airflow/models/taskinstance.py
index 4fd72e7..b4e644e 100644
--- a/airflow/models/taskinstance.py
+++ b/airflow/models/taskinstance.py
@@ -1087,12 +1087,23 @@ class TaskInstance(Base, LoggingMixin): # pylint:
disable=R0902,R0904
self.log.info("Executing %s on %s", self.task,
self.execution_date)
return True
-def _date_or_empty(self, attr):
-if hasattr(self, attr):
-date = getattr(self, attr)
-if date:
-return date.strftime('%Y%m%dT%H%M%S')
-return ''
+def _date_or_empty(self, attr: str):
+result = getattr(self, attr, None) # type: datetime
+return result.strftime('%Y%m%dT%H%M%S') if result else ''
+
+def _log_state(self, lead_msg: str = ''):
+self.log.info(
+'%sMarking task as %s.'
++ ' dag_id=%s, task_id=%s,'
++ ' execution_date=%s, start_date=%s, end_date=%s',
+lead_msg,
+self.state.upper(),
+self.dag_id,
+self.task_id,
+self._date_or_empty('execution_date'),
+self._date_or_empty('start_date'),
+self._date_or_empty('end_date'),
+)
@provide_session
@Sentry.enrich_errors
@@ -1147,15 +1158,6 @@ class TaskInstance(Base, LoggingMixin): # pylint:
disable=R0902,R0904
self.log.info(e)
self.refresh_from_db(lock_for_update=True)
self.state = State.SKIPPED
-self.log.info(
-'Marking task as SKIPPED. '
-'dag_id=%s, task_id=%s, execution_date=%s, start_date=%s,
end_date=%s',
-self.dag_id,
-self.task_id,
-self._date_or_empty('execution_date'),
-self._date_or_empty('start_date'),
-self._date_or_empty('end_date'),
-)
except AirflowRescheduleException as reschedule_exception:
self.refresh_from_db()
self._handle_reschedule(actual_start_date, reschedule_exception,
test_mode)
@@ -1181,17 +1183,9 @@ class TaskInstance(Base, LoggingMixin): # pylint:
disable=R0902,R0904
finally:
Stats.incr(f'ti.finish.{task.dag_id}.{task.task_id}.{self.state}')
-# Recording SUCCESS
+# Recording SKIPPED or SUCCESS
self.end_date = timezone.utcnow()
-self.log.info(
-'Marking task as SUCCESS. '
-'dag_id=%s, task_id=%s, execution_date=%s, start_date=%s,
end_date=%s',
-self.dag_id,
-self.task_id,
-self._date_or_empty('execution_date'),
-self._date_or_empty('start_date'),
-self._date_or_empty('end_date'),
-)
+self._log_state()
self.set_duration()
if not test_mode:
session.add(Log(self.state, self))
@@ -1458,25 +1452,12 @@ class TaskInstance(Base, LoggingMixin): # pylint:
disable=R0902,R0904
if force_fail or not self.is_eligible_to_retry():
self.state = State.FAILED
-if force_fail:
-log_message = "Immediate failure requested. Marking task as
FAILED."
-else:
-log_message = "Marking task as FAILED."
email_for_state = task.email_on_failure
else:
self.state = State.UP_FOR_RETRY
-log_message = "Marking task as UP_FOR_RETRY."
email_for_state = task.email_on_retry
-self.log.info(
-'%s dag_id=%s, task_id=%s, execution_date=%s, start_date=%s,
end_date=%s',
-log_message,
-self.dag_id,
-self.task_id,
-self._safe_date('execution_date', '%Y%m%dT%H%M%S'),
-self._safe_date('start_date', '%Y%m%dT%H%M%S'),
-self._safe_date('end_date', '%Y%m%dT%H%M%S'),
-)
+self._log_state('Immediate failure requested. ' if force_fail else '')
if email_for_state and task.email:
try:
self.email_alert(error)
@@
[GitHub] [airflow] kaxil merged pull request #14822: `SKIPPED` shouldn't be logged again as `SUCCESS`.
kaxil merged pull request #14822: URL: https://github.com/apache/airflow/pull/14822 -- This is an automated message from the 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 main updated: Fix broken static checks from #15915 (#16378)
This is an automated email from the ASF dual-hosted git repository.
kaxilnaik pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new f2315bf Fix broken static checks from #15915 (#16378)
f2315bf is described below
commit f2315bf31b513c18eb266467f6c3036351d15050
Author: Jed Cunningham <66968678+jedcunning...@users.noreply.github.com>
AuthorDate: Thu Jun 10 15:41:47 2021 -0600
Fix broken static checks from #15915 (#16378)
---
airflow/www/views.py | 17 -
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/airflow/www/views.py b/airflow/www/views.py
index cdee2b9..e7eeb3c 100644
--- a/airflow/www/views.py
+++ b/airflow/www/views.py
@@ -2310,16 +2310,10 @@ class Airflow(AirflowBaseView): # noqa: D101 pylint:
disable=too-many-public-m
dag = dag.partial_subset(task_ids_or_regex=root,
include_upstream=True, include_downstream=False)
chart_height = wwwutils.get_chart_height(dag)
chart = nvd3.lineChart(
-name="lineChart",
-x_is_date=True,
-height=chart_height,
-chart_attr=self.line_chart_attr
+name="lineChart", x_is_date=True, height=chart_height,
chart_attr=self.line_chart_attr
)
cum_chart = nvd3.lineChart(
-name="cumLineChart",
-x_is_date=True,
-height=chart_height,
-chart_attr=self.line_chart_attr
+name="cumLineChart", x_is_date=True, height=chart_height,
chart_attr=self.line_chart_attr
)
y_points = defaultdict(list)
@@ -2443,7 +2437,7 @@ class Airflow(AirflowBaseView): # noqa: D101 pylint:
disable=too-many-public-m
x_is_date=True,
y_axis_format='d',
height=chart_height,
-chart_attr=self.line_chart_attr
+chart_attr=self.line_chart_attr,
)
for task in dag.tasks:
@@ -2514,10 +2508,7 @@ class Airflow(AirflowBaseView): # noqa: D101 pylint:
disable=too-many-public-m
chart_height = wwwutils.get_chart_height(dag)
chart = nvd3.lineChart(
-name="lineChart",
-x_is_date=True,
-height=chart_height,
-chart_attr=self.line_chart_attr
+name="lineChart", x_is_date=True, height=chart_height,
chart_attr=self.line_chart_attr
)
y_points = {}
x_points = {}
[GitHub] [airflow] kaxil merged pull request #16378: Fix broken static checks from #15915
kaxil merged pull request #16378: URL: https://github.com/apache/airflow/pull/16378 -- This is an automated message from the 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] github-actions[bot] commented on pull request #16378: Fix broken static checks from #15915
github-actions[bot] commented on pull request #16378: URL: https://github.com/apache/airflow/pull/16378#issuecomment-859095470 The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease. -- This is an automated message from the 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] jedcunningham opened a new pull request #16378: Fix broken static checks from #15915
jedcunningham opened a new pull request #16378: URL: https://github.com/apache/airflow/pull/16378 This fixes the static checks in main that were accidentally broken in #15915. -- This is an automated message from the 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] jedcunningham commented on issue #16060: apply_defaults doesn't run for decorated task
jedcunningham commented on issue #16060: URL: https://github.com/apache/airflow/issues/16060#issuecomment-859084846 @Junnplus, did #16085 fix this issue? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] jhtimmins merged pull request #16377: Swap out calls to find_permission_view_menu for get_permission wrapper.
jhtimmins merged pull request #16377: URL: https://github.com/apache/airflow/pull/16377 -- This is an automated message from the 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 main updated: Swap out calls to find_permission_view_menu for get_permission wrapper. (#16377)
This is an automated email from the ASF dual-hosted git repository.
jhtimmins pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 11cf6f3 Swap out calls to find_permission_view_menu for
get_permission wrapper. (#16377)
11cf6f3 is described below
commit 11cf6f3ef4f7e39c3a634cb17dc50c190dbee582
Author: James Timmins
AuthorDate: Thu Jun 10 14:19:39 2021 -0700
Swap out calls to find_permission_view_menu for get_permission wrapper.
(#16377)
---
.../versions/2c6edca13270_resource_based_permissions.py| 4 ++--
.../82b7c48c147f_remove_can_read_permission_on_config_.py | 4 ++--
...a13f7613ad25_resource_based_permissions_for_default_.py | 4 ++--
airflow/www/security.py| 2 +-
tests/test_utils/api_connexion_utils.py| 2 +-
tests/www/test_security.py | 8 +++-
tests/www/views/test_views_acl.py | 14 +++---
7 files changed, 18 insertions(+), 20 deletions(-)
diff --git
a/airflow/migrations/versions/2c6edca13270_resource_based_permissions.py
b/airflow/migrations/versions/2c6edca13270_resource_based_permissions.py
index fdba3f9..54b397a 100644
--- a/airflow/migrations/versions/2c6edca13270_resource_based_permissions.py
+++ b/airflow/migrations/versions/2c6edca13270_resource_based_permissions.py
@@ -289,7 +289,7 @@ def remap_permissions():
appbuilder = create_app(config={'FAB_UPDATE_PERMS': False}).appbuilder
for old, new in mapping.items():
(old_view_name, old_perm_name) = old
-old_pvm = appbuilder.sm.find_permission_view_menu(old_perm_name,
old_view_name)
+old_pvm = appbuilder.sm.get_permission(old_perm_name, old_view_name)
if not old_pvm:
continue
for new_perm_name, new_view_name in new:
@@ -303,7 +303,7 @@ def remap_permissions():
if not appbuilder.sm.find_permission(old_perm_name):
continue
view_menus = appbuilder.sm.get_all_view_menu()
-if not any(appbuilder.sm.find_permission_view_menu(old_perm_name,
view.name) for view in view_menus):
+if not any(appbuilder.sm.get_permission(old_perm_name, view.name) for
view in view_menus):
appbuilder.sm.del_permission(old_perm_name)
diff --git
a/airflow/migrations/versions/82b7c48c147f_remove_can_read_permission_on_config_.py
b/airflow/migrations/versions/82b7c48c147f_remove_can_read_permission_on_config_.py
index 85d0872..8803c87 100644
---
a/airflow/migrations/versions/82b7c48c147f_remove_can_read_permission_on_config_.py
+++
b/airflow/migrations/versions/82b7c48c147f_remove_can_read_permission_on_config_.py
@@ -42,7 +42,7 @@ def upgrade():
appbuilder = create_app(config={'FAB_UPDATE_PERMS': False}).appbuilder
roles_to_modify = [role for role in appbuilder.sm.get_all_roles() if
role.name in ["User", "Viewer"]]
-can_read_on_config_perm = appbuilder.sm.find_permission_view_menu(
+can_read_on_config_perm = appbuilder.sm.get_permission(
permissions.ACTION_CAN_READ, permissions.RESOURCE_CONFIG
)
@@ -59,7 +59,7 @@ def downgrade():
"""Add can_read permission on config resource for User and Viewer role"""
appbuilder = create_app(config={'FAB_UPDATE_PERMS': False}).appbuilder
roles_to_modify = [role for role in appbuilder.sm.get_all_roles() if
role.name in ["User", "Viewer"]]
-can_read_on_config_perm = appbuilder.sm.find_permission_view_menu(
+can_read_on_config_perm = appbuilder.sm.get_permission(
permissions.ACTION_CAN_READ, permissions.RESOURCE_CONFIG
)
diff --git
a/airflow/migrations/versions/a13f7613ad25_resource_based_permissions_for_default_.py
b/airflow/migrations/versions/a13f7613ad25_resource_based_permissions_for_default_.py
index bf86839..c918b30 100644
---
a/airflow/migrations/versions/a13f7613ad25_resource_based_permissions_for_default_.py
+++
b/airflow/migrations/versions/a13f7613ad25_resource_based_permissions_for_default_.py
@@ -141,7 +141,7 @@ def remap_permissions():
appbuilder = create_app(config={'FAB_UPDATE_PERMS': False}).appbuilder
for old, new in mapping.items():
(old_view_name, old_perm_name) = old
-old_pvm = appbuilder.sm.find_permission_view_menu(old_perm_name,
old_view_name)
+old_pvm = appbuilder.sm.get_permission(old_perm_name, old_view_name)
if not old_pvm:
continue
for new_perm_name, new_view_name in new:
@@ -155,7 +155,7 @@ def remap_permissions():
if not appbuilder.sm.find_permission(old_perm_name):
continue
view_menus = appbuilder.sm.get_all_view_menu()
-if not any(appbuilder.sm.find_permission_view_menu(old_perm_name,
view.name) for view in view_menus):
+if not any(appbuilder.sm.get_permission(old_perm_name, view.name) for
view in view_menus):
[airflow] 01/03: Ensure that we don't try to mask empty string in logs (#16057)
This is an automated email from the ASF dual-hosted git repository. jhtimmins pushed a commit to branch v2-1-test in repository https://gitbox.apache.org/repos/asf/airflow.git commit 789aacea36820e1e72c7962b539d57c36134910c Author: Ash Berlin-Taylor AuthorDate: Tue May 25 19:31:22 2021 +0100 Ensure that we don't try to mask empty string in logs (#16057) Although `Connection.password` being empty was guarded against, there are other possible cases (such as an extra field) that wasn't guarded against, which ended up with this in the logs: WARNING - ***-***-***-*** ***L***o***g***g***i***n***g*** ***e***r***r***o***r*** ***-***-***-*** Oops! (cherry picked from commit 8814a59a5bf54dd17aef21eefd0900703330c22c)
[airflow] branch v2-1-test updated (779811e -> abc86c4)
This is an automated email from the ASF dual-hosted git repository. jhtimmins pushed a change to branch v2-1-test in repository https://gitbox.apache.org/repos/asf/airflow.git. from 779811e Fill the "job_id" field for `airflow task run` without `--local`/`--raw` for KubeExecutor (#16108) new 789aace Ensure that we don't try to mask empty string in logs (#16057) new fb714d6 set max tree width to 1200px (#16067) new abc86c4 Don't fail to log if we can't redact something (#16118) The 3 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: airflow/utils/log/secrets_masker.py| 53 ++ airflow/www/static/js/tree.js | 4 ++- tests/utils/log/test_secrets_masker.py | 24 +++ 3 files changed, 62 insertions(+), 19 deletions(-)
[airflow] 03/03: Don't fail to log if we can't redact something (#16118)
This is an automated email from the ASF dual-hosted git repository.
jhtimmins pushed a commit to branch v2-1-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
commit abc86c4bd96318b40615f8f173e08208a4756905
Author: Ash Berlin-Taylor
AuthorDate: Mon Jun 7 09:27:01 2021 +0100
Don't fail to log if we can't redact something (#16118)
Rather than dying with an exception, catch it and warn about that,
asking users to report it to us.
Additionally handle the specific case where a file handle/IO object is
logged -- we definitely don't want to iterate over that!
(cherry picked from commit 57bd6fb2925a7d505a80b83140811b94b363f49c)
---
airflow/utils/log/secrets_masker.py| 53 ++
tests/utils/log/test_secrets_masker.py | 24 +++
2 files changed, 59 insertions(+), 18 deletions(-)
diff --git a/airflow/utils/log/secrets_masker.py
b/airflow/utils/log/secrets_masker.py
index 6df8d39..b3ccfdb 100644
--- a/airflow/utils/log/secrets_masker.py
+++ b/airflow/utils/log/secrets_masker.py
@@ -16,6 +16,7 @@
# under the License.
"""Mask sensitive information from logs"""
import collections
+import io
import logging
import re
from typing import TYPE_CHECKING, Iterable, Optional, Set, TypeVar, Union
@@ -40,6 +41,10 @@ if TYPE_CHECKING:
RedactableItem = TypeVar('RedctableItem')
+
+log = logging.getLogger(__name__)
+
+
DEFAULT_SENSITIVE_FIELDS = frozenset(
{
'password',
@@ -186,24 +191,36 @@ class SecretsMasker(logging.Filter):
is redacted.
"""
-if name and should_hide_value_for_key(name):
-return self._redact_all(item)
-
-if isinstance(item, dict):
-return {dict_key: self.redact(subval, dict_key) for dict_key,
subval in item.items()}
-elif isinstance(item, str):
-if self.replacer:
-# We can't replace specific values, but the key-based redacting
-# can still happen, so we can't short-circuit, we need to walk
-# the structure.
-return self.replacer.sub('***', item)
-return item
-elif isinstance(item, (tuple, set)):
-# Turn set in to tuple!
-return tuple(self.redact(subval) for subval in item)
-elif isinstance(item, Iterable):
-return list(self.redact(subval) for subval in item)
-else:
+try:
+if name and should_hide_value_for_key(name):
+return self._redact_all(item)
+
+if isinstance(item, dict):
+return {dict_key: self.redact(subval, dict_key) for dict_key,
subval in item.items()}
+elif isinstance(item, str):
+if self.replacer:
+# We can't replace specific values, but the key-based
redacting
+# can still happen, so we can't short-circuit, we need to
walk
+# the structure.
+return self.replacer.sub('***', item)
+return item
+elif isinstance(item, (tuple, set)):
+# Turn set in to tuple!
+return tuple(self.redact(subval) for subval in item)
+elif isinstance(item, io.IOBase):
+return item
+elif isinstance(item, Iterable):
+return list(self.redact(subval) for subval in item)
+else:
+return item
+except Exception as e: # pylint: disable=broad-except
+log.warning(
+"Unable to redact %r, please report this via
. "
+"Error was: %s: %s",
+item,
+type(e).__name__,
+str(e),
+)
return item
# pylint: enable=too-many-return-statements
diff --git a/tests/utils/log/test_secrets_masker.py
b/tests/utils/log/test_secrets_masker.py
index 8c88bdd..24e86c1 100644
--- a/tests/utils/log/test_secrets_masker.py
+++ b/tests/utils/log/test_secrets_masker.py
@@ -72,6 +72,22 @@ class TestSecretsMasker:
assert caplog.text == "INFO Cannot connect to user:***\n"
+def test_non_redactable(self, logger, caplog):
+class NonReactable:
+def __iter__(self):
+raise RuntimeError("force fail")
+
+def __repr__(self):
+return ""
+
+logger.info("Logging %s", NonReactable())
+
+assert caplog.messages == [
+"Unable to redact , please report this via "
++ ". Error was:
RuntimeError: force fail",
+"Logging ",
+]
+
def test_extra(self, logger, caplog):
logger.handlers[0].formatter = ShortExcFormatter("%(levelname)s
%(message)s %(conn)s")
logger.info("Cannot connect", extra={'conn': "user:password"})
@@ -202,6 +218,14 @@ class TestSecretsMasker:
[airflow] 02/03: set max tree width to 1200px (#16067)
This is an automated email from the ASF dual-hosted git repository.
jhtimmins pushed a commit to branch v2-1-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
commit fb714d6db9e28de113014364365202c82eb0ce22
Author: Brent Bovenzi
AuthorDate: Tue May 25 16:20:31 2021 -0400
set max tree width to 1200px (#16067)
the totalwidth of the tree view will depend on the window size like before,
but max out at 1200px
(cherry picked from commit f2aa9b58cb012a3bc347f43baeaa41ecdece4cbf)
---
airflow/www/static/js/tree.js | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/airflow/www/static/js/tree.js b/airflow/www/static/js/tree.js
index 04702a7..cc8276d 100644
--- a/airflow/www/static/js/tree.js
+++ b/airflow/www/static/js/tree.js
@@ -70,7 +70,9 @@ document.addEventListener('DOMContentLoaded', () => {
if (node.depth > treeDepth) treeDepth = node.depth;
});
treeDepth += 1;
- const squareX = window.innerWidth - (data.instances.length * squareSize) -
(treeDepth * 50);
+
+ const innerWidth = window.innerWidth > 1200 ? 1200 : window.innerWidth;
+ const squareX = innerWidth - (data.instances.length * squareSize) -
(treeDepth * 50);
const squareSpacing = 2;
const margin = {
[GitHub] [airflow] ephraimbuddy commented on pull request #16318: Fix API List DAGs inconsistent with UI/CLI
ephraimbuddy commented on pull request #16318: URL: https://github.com/apache/airflow/pull/16318#issuecomment-859034744 Congrats on the PR @jpyen -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #16376: Set num of retries for google cloud connection to 3.
mik-laj commented on a change in pull request #16376:
URL: https://github.com/apache/airflow/pull/16376#discussion_r649480136
##
File path: airflow/utils/db.py
##
@@ -277,6 +277,7 @@ def create_default_connections(session=None):
conn_id="google_cloud_default",
conn_type="google_cloud_platform",
schema="default",
+extra='{"extra__google_cloud_platform__num_retries": 3}',
Review comment:
By default, we set this value to 5. See:
https://github.com/apache/airflow/blob/491c835051e904d7632b05bc3512a7cb77e3b575/airflow/providers/google/common/hooks/base_google.py#L189
https://github.com/apache/airflow/blob/491c835051e904d7632b05bc3512a7cb77e3b575/airflow/providers/google/common/hooks/base_google.py#L310-L330
You don't need to configure it again here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #16376: Set num of retries for google cloud connection to 3.
mik-laj commented on a change in pull request #16376:
URL: https://github.com/apache/airflow/pull/16376#discussion_r649480136
##
File path: airflow/utils/db.py
##
@@ -277,6 +277,7 @@ def create_default_connections(session=None):
conn_id="google_cloud_default",
conn_type="google_cloud_platform",
schema="default",
+extra='{"extra__google_cloud_platform__num_retries": 3}',
Review comment:
By default, we set this value to 5. See:
https://github.com/apache/airflow/blob/491c835051e904d7632b05bc3512a7cb77e3b575/airflow/providers/google/common/hooks/base_google.py#L189
https://github.com/apache/airflow/blob/491c835051e904d7632b05bc3512a7cb77e3b575/airflow/providers/google/common/hooks/base_google.py#L310-L330
--
This is an automated message from the 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 #16345: Correctly handle None returns from Query.scalar()
kaxil commented on a change in pull request #16345:
URL: https://github.com/apache/airflow/pull/16345#discussion_r649475256
##
File path: airflow/jobs/scheduler_job.py
##
@@ -1748,6 +1748,8 @@ def _schedule_dag_run(
def _verify_integrity_if_dag_changed(self, dag_run: DagRun, session=None):
"""Only run DagRun.verify integrity if Serialized DAG has changed
since it is slow"""
latest_version =
SerializedDagModel.get_latest_version_hash(dag_run.dag_id, session=session)
+if latest_version is None:
+raise AirflowException(f"Serialized DAG not found for DAG run
{dag_run.id}.")
Review comment:
```suggestion
raise SerializedDagNotFound(f"Serialized DAG not found for DAG
run {dag_run.id}.")
```
This is because we handle it below:
https://github.com/apache/airflow/blob/491c835051e904d7632b05bc3512a7cb77e3b575/airflow/jobs/scheduler_job.py#L1496-L1500
--
This is an automated message from the 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] github-actions[bot] commented on pull request #16377: Swap out calls to find_permission_view_menu for get_permission wrapper.
github-actions[bot] commented on pull request #16377: URL: https://github.com/apache/airflow/pull/16377#issuecomment-858971855 The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease. -- This is an automated message from the 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 #16345: Correctly handle None returns from Query.scalar()
kaxil commented on a change in pull request #16345:
URL: https://github.com/apache/airflow/pull/16345#discussion_r649475256
##
File path: airflow/jobs/scheduler_job.py
##
@@ -1748,6 +1748,8 @@ def _schedule_dag_run(
def _verify_integrity_if_dag_changed(self, dag_run: DagRun, session=None):
"""Only run DagRun.verify integrity if Serialized DAG has changed
since it is slow"""
latest_version =
SerializedDagModel.get_latest_version_hash(dag_run.dag_id, session=session)
+if latest_version is None:
+raise AirflowException(f"Serialized DAG not found for DAG run
{dag_run.id}.")
Review comment:
```suggestion
raise SerializedDagNotFound(f"Serialized DAG not found for DAG
run {dag_run.id}.")
```
--
This is an automated message from the 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] jhtimmins opened a new pull request #16377: Swap out calls to find_permission_view_menu for get_permission wrapper.
jhtimmins opened a new pull request #16377: URL: https://github.com/apache/airflow/pull/16377 Part of the migration to the resource->action Permission naming scheme. Swap out calls find_permission_view_menu for it's wrapper method, get_permission. -- This is an automated message from the 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 main updated (9ba796e -> 491c835)
This is an automated email from the ASF dual-hosted git repository. jhtimmins pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/airflow.git. from 9ba796e Make REST API List DAGs endpoint consistent with UI/CLI behaviour (#16318) add 491c835 Docs: Change 10 minutes to 100 minutes in ``worker_refresh_interval`` (#16369) No new revisions were added by this update. Summary of changes: UPDATING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
[GitHub] [airflow] jhtimmins merged pull request #16369: Docs: Change 10 minutes to 100 minutes in ``worker_refresh_interval``
jhtimmins merged pull request #16369: URL: https://github.com/apache/airflow/pull/16369 -- This is an automated message from the 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] sshah90 commented on pull request #12925: [Loki log handler] - Integration with Grafana Loki
sshah90 commented on pull request #12925: URL: https://github.com/apache/airflow/pull/12925#issuecomment-858928062 @ashb wondering if I can add hash key of `dag_name + task_name + execution_date + try_number` in each log stream with ` setFormatter(self.formatter)` function and filter on that while reading? -- This is an automated message from the 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 #16343: Fix DAG run state not updated while DAG is paused
ephraimbuddy commented on a change in pull request #16343: URL: https://github.com/apache/airflow/pull/16343#discussion_r649449800 ## File path: airflow/config_templates/default_airflow.cfg ## @@ -849,6 +849,10 @@ job_heartbeat_sec = 5 # that no longer have a matching DagRun clean_tis_without_dagrun_interval = 15.0 +# How often (in seconds) to check paused DAGs for DagRuns in running state +# and update states of DagRuns whose tasks finished the DagRun +update_dagrun_state_for_paused_dag_interval = 30.0 + Review comment: ```suggestion ``` -- This is an automated message from the 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 #16343: Fix DAG run state not updated while DAG is paused
ephraimbuddy commented on a change in pull request #16343: URL: https://github.com/apache/airflow/pull/16343#discussion_r649449578 ## File path: airflow/config_templates/config.yml ## @@ -1704,6 +1704,14 @@ type: float example: ~ default: "15.0" +- name: update_dagrun_state_for_paused_dag_interval + description: | +How often (in seconds) to check paused DAGs for DagRuns in running state +and update states of DagRuns whose tasks finished the DagRun + version_added: 2.1.1 + type: float + example: ~ + default: "30.0" Review comment: ```suggestion ``` -- This is an automated message from the 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] eladkal commented on issue #16135: Associate DagRun to Kubernetes Pod
eladkal commented on issue #16135: URL: https://github.com/apache/airflow/issues/16135#issuecomment-858907871 @dimberman @jedcunningham WDYT? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] XD-DENG commented on pull request #16358: Only create dagruns when max_active_runs is not reached
XD-DENG commented on pull request #16358: URL: https://github.com/apache/airflow/pull/16358#issuecomment-858887591 I haven't looked into the details. But based on the idea I get from the PR subject, this may affect the DAGs whose 'catchup' is False -- This is an automated message from the 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 #16110: Added windows extensions
ashb commented on a change in pull request #16110:
URL: https://github.com/apache/airflow/pull/16110#discussion_r649426106
##
File path: airflow/__main__.py
##
@@ -34,6 +35,15 @@ def main():
os.environ['KRB5CCNAME'] = conf.get('kerberos', 'ccache')
os.environ['KRB5_KTNAME'] = conf.get('kerberos', 'keytab')
+# if dags folder has to be set to configured value, make sure it is set
properly (needed on Dask-Workers)
Review comment:
This is not the right fix for this.
There's a bug in a previous feature where the "dag folder" should be
replaced with `DAG_FOLDER` which is then automaticall replaced, but this isn't
working.
We should fix that rather than adding a new config and new way of making
this work.
See
https://github.com/apache/airflow/blob/9ba796ef40fe833aba58f5aa13a63587106d8ffd/airflow/utils/cli.py#L160-L167
for where the code is (the problem is on the command we send to the executor.)
https://github.com/apache/airflow/issues/8061
--
This is an automated message from the 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 #16110: Added windows extensions
ashb commented on a change in pull request #16110:
URL: https://github.com/apache/airflow/pull/16110#discussion_r649426106
##
File path: airflow/__main__.py
##
@@ -34,6 +35,15 @@ def main():
os.environ['KRB5CCNAME'] = conf.get('kerberos', 'ccache')
os.environ['KRB5_KTNAME'] = conf.get('kerberos', 'keytab')
+# if dags folder has to be set to configured value, make sure it is set
properly (needed on Dask-Workers)
Review comment:
This is not the right fix for this.
There's a bug in a previous feature where the "dag folder" should be
replaced with `DAG_FOLDER` which is then automaticall replaced, but this isn't
working.
We should fix that rather than adding a new config and new way of making
this work.
See
https://github.com/apache/airflow/blob/9ba796ef40fe833aba58f5aa13a63587106d8ffd/airflow/utils/cli.py#L160-L167
for where the code is (the problem is on the command we send to the executor.)
--
This is an automated message from the 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] ManiBharataraju commented on issue #14231: Quick start for Airflow on Mac OS
ManiBharataraju commented on issue #14231: URL: https://github.com/apache/airflow/issues/14231#issuecomment-858878483 @mik-laj - I somehow managed to do all the steps in my mac to get the UI up but when I go to the UI I am unable to click on anything and see something weird like this. Could you please let me know what has gone wrong or what is missed?  -- This is an automated message from the 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 #16110: Added windows extensions
ashb commented on a change in pull request #16110:
URL: https://github.com/apache/airflow/pull/16110#discussion_r649426106
##
File path: airflow/__main__.py
##
@@ -34,6 +35,15 @@ def main():
os.environ['KRB5CCNAME'] = conf.get('kerberos', 'ccache')
os.environ['KRB5_KTNAME'] = conf.get('kerberos', 'keytab')
+# if dags folder has to be set to configured value, make sure it is set
properly (needed on Dask-Workers)
Review comment:
This is not the right fix for this.
There's a bug in a previous feature where the "dag folder" should be
replaced with `DAG_FOLDER` which is then automaticall replaced, but this isn't
working.
We should fix that rather than adding a new config and new way of making 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 #16089: [#16087]: Backfill shouldn't interfere with scheduled run fix
ashb commented on a change in pull request #16089: URL: https://github.com/apache/airflow/pull/16089#discussion_r649424052 ## File path: airflow/jobs/backfill_job.py ## @@ -689,6 +688,9 @@ def tabulate_tis_set(set_tis: Set[TaskInstance]) -> str: return err +def _get_dag_with_subdags(self): +return [self.dag] + self.dag.subdags + Review comment: ```suggestion def _get_dag_with_subdags(self): return [self.dag] + self.dag.subdags ``` Keep it the way it was -- adding a one line function that is only called in one place doesn't make readability any better. -- This is an automated message from the 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] jpyen commented on pull request #16318: Fix API List DAGs inconsistent with UI/CLI
jpyen commented on pull request #16318: URL: https://github.com/apache/airflow/pull/16318#issuecomment-858873132 Thanks @ephraimbuddy! -- This is an automated message from the 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] eladkal commented on a change in pull request #16365: Implemented Tableau Hook connection through SSL
eladkal commented on a change in pull request #16365:
URL: https://github.com/apache/airflow/pull/16365#discussion_r649417418
##
File path: airflow/providers/tableau/hooks/tableau.py
##
@@ -61,7 +61,9 @@ def __init__(self, site_id: Optional[str] = None,
tableau_conn_id: str = default
self.tableau_conn_id = tableau_conn_id
self.conn = self.get_connection(self.tableau_conn_id)
self.site_id = site_id or self.conn.extra_dejson.get('site_id', '')
-self.server = Server(self.conn.host, use_server_version=True)
+self.server = Server(self.conn.host)
+self.server.add_http_options(options_dict={'verify':
self.conn.extra_dejson.get('verify', False)})
Review comment:
if I read the tableau lib code correctly it should be possible to set
both verify and cert yet with this change you allow only verify.
Some like how it is used at:
https://community.tableau.com/s/question/0D54T0F33bd/tableauserverclient-signin-with-ssl-certificate
Also is it smart to keep the default`False`? WDYT?
**Edit:** Also I'm a bit confused according to the library example
https://github.com/tableau/server-client-python/blob/ce37a2063eddf317aaf2703aa3ebccf8053e1c8a/samples/set_http_options.py#L35:L36
the default is actually `True` so without the changes of this PR you should
have `verify=True` by default. isn't 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] eladkal commented on a change in pull request #16365: Implemented Tableau Hook connection through SSL
eladkal commented on a change in pull request #16365:
URL: https://github.com/apache/airflow/pull/16365#discussion_r649417418
##
File path: airflow/providers/tableau/hooks/tableau.py
##
@@ -61,7 +61,9 @@ def __init__(self, site_id: Optional[str] = None,
tableau_conn_id: str = default
self.tableau_conn_id = tableau_conn_id
self.conn = self.get_connection(self.tableau_conn_id)
self.site_id = site_id or self.conn.extra_dejson.get('site_id', '')
-self.server = Server(self.conn.host, use_server_version=True)
+self.server = Server(self.conn.host)
+self.server.add_http_options(options_dict={'verify':
self.conn.extra_dejson.get('verify', False)})
Review comment:
if I read the tableau lib code correctly it should be possible to set
both verify and cert yet with this change you allow only verify.
Some like how it is used at:
https://community.tableau.com/s/question/0D54T0F33bd/tableauserverclient-signin-with-ssl-certificate
Also is it smart to keep the default`False`? WDYT?
Edit: Also I'm a bit confused according to the library example
https://github.com/tableau/server-client-python/blob/ce37a2063eddf317aaf2703aa3ebccf8053e1c8a/samples/set_http_options.py#L35:L36
the default is actually `True` so without the changes of this PR you should
have `verify=True` by default. isn't 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] eladkal commented on a change in pull request #16365: Implemented Tableau Hook connection through SSL
eladkal commented on a change in pull request #16365:
URL: https://github.com/apache/airflow/pull/16365#discussion_r649417418
##
File path: airflow/providers/tableau/hooks/tableau.py
##
@@ -61,7 +61,9 @@ def __init__(self, site_id: Optional[str] = None,
tableau_conn_id: str = default
self.tableau_conn_id = tableau_conn_id
self.conn = self.get_connection(self.tableau_conn_id)
self.site_id = site_id or self.conn.extra_dejson.get('site_id', '')
-self.server = Server(self.conn.host, use_server_version=True)
+self.server = Server(self.conn.host)
+self.server.add_http_options(options_dict={'verify':
self.conn.extra_dejson.get('verify', False)})
Review comment:
if I read the tableau lib code correctly it should be possible to set
both verify and cert yet with this change you allow only verify.
Some like how it is used at:
https://community.tableau.com/s/question/0D54T0F33bd/tableauserverclient-signin-with-ssl-certificate
Also is it smart to keep the default`False`? WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] ashb commented on issue #16303: Replace `execution_date` in `TI.generate_command` to send run_id instead
ashb commented on issue #16303: URL: https://github.com/apache/airflow/issues/16303#issuecomment-858868501 @SamWheating See also https://github.com/apache/airflow/pull/7085 -- you might want to fold that in to this change. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[airflow] branch main updated (9351e2a -> 9ba796e)
This is an automated email from the ASF dual-hosted git repository. ephraimanierobi pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/airflow.git. from 9351e2a Depreciate private_key_pass in SFTPHook conn extra and rename to private_key_passphrase (#14028) add 9ba796e Make REST API List DAGs endpoint consistent with UI/CLI behaviour (#16318) No new revisions were added by this update. Summary of changes: airflow/api_connexion/endpoints/dag_endpoint.py| 14 ++ tests/api_connexion/endpoints/test_dag_endpoint.py | 19 ++- 2 files changed, 28 insertions(+), 5 deletions(-)
[GitHub] [airflow] eladkal commented on a change in pull request #16365: Implemented Tableau Hook connection through SSL
eladkal commented on a change in pull request #16365:
URL: https://github.com/apache/airflow/pull/16365#discussion_r649417418
##
File path: airflow/providers/tableau/hooks/tableau.py
##
@@ -61,7 +61,9 @@ def __init__(self, site_id: Optional[str] = None,
tableau_conn_id: str = default
self.tableau_conn_id = tableau_conn_id
self.conn = self.get_connection(self.tableau_conn_id)
self.site_id = site_id or self.conn.extra_dejson.get('site_id', '')
-self.server = Server(self.conn.host, use_server_version=True)
+self.server = Server(self.conn.host)
+self.server.add_http_options(options_dict={'verify':
self.conn.extra_dejson.get('verify', False)})
Review comment:
if I read the tableau lib code correctly it should be possible to set
both verify and cert yet with this change you allow only verify.
Some like how it is used at:
https://community.tableau.com/s/question/0D54T0F33bd/tableauserverclient-signin-with-ssl-certificate
Also why the default is `False`?
--
This is an automated message from the 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 merged pull request #16318: Fix API List DAGs inconsistent with UI/CLI
ephraimbuddy merged pull request #16318: URL: https://github.com/apache/airflow/pull/16318 -- This is an automated message from the 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 #15980: Always return a response in TI's action_clear view
ashb commented on pull request #15980: URL: https://github.com/apache/airflow/pull/15980#issuecomment-858862521 @uranusjr I'm seeing this in the logs: ``` tests/www/views/test_views_tasks.py::test_task_instance_clear_failure XFAIL (until #15980 is merged) [ 72%] ``` Shouldn't we expect this to pass 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] jpyen edited a comment on pull request #16318: Fix API List DAGs inconsistent with UI/CLI
jpyen edited a comment on pull request #16318: URL: https://github.com/apache/airflow/pull/16318#issuecomment-858817951 @kaxil @ephraimbuddy Changes approved, checks passed and ready to merge. Thanks for the patience. -- This is an automated message from the 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] github-actions[bot] commented on pull request #16375: Fix normalize-url vulnerability
github-actions[bot] commented on pull request #16375: URL: https://github.com/apache/airflow/pull/16375#issuecomment-858847814 The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease. -- This is an automated message from the 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 closed pull request #8651: [AIRFLOW-8058] Add configurable execution context
ashb closed pull request #8651: URL: https://github.com/apache/airflow/pull/8651 -- This is an automated message from the 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 #8651: [AIRFLOW-8058] Add configurable execution context
ashb commented on pull request #8651: URL: https://github.com/apache/airflow/pull/8651#issuecomment-858847804 I'm afraid I'm going to close this as Wont Fix -- it feels very broad and fragile to use this global config setting for the example you've given (GCP project) This is also not necessary for the original target issue There is already a `on_execute_callback`, so perhaps a much simpler approach might be to add a matching `on_post_execute_callback` (PR) and then you can use https://airflow.apache.org/docs/apache-airflow/stable/concepts/cluster-policies.html?highlight=policy to set this globally in our install. Sorry to leave this PR mostly silent for most of a year to then close it, but we can build a simpler 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] msumit commented on a change in pull request #16233: Fix TI success/failure links
msumit commented on a change in pull request #16233:
URL: https://github.com/apache/airflow/pull/16233#discussion_r649408501
##
File path: airflow/www/views.py
##
@@ -1803,54 +1804,104 @@ def _mark_task_instance_state( # pylint:
disable=too-many-arguments
latest_execution_date = dag.get_latest_execution_date()
if not latest_execution_date:
-flash(f"Cannot make {state}, seem that dag {dag_id} has never
run", "error")
+flash(f"Cannot mark tasks as {state}, seem that dag {dag_id} has
never run", "error")
return redirect(origin)
execution_date = timezone.parse(execution_date)
from airflow.api.common.experimental.mark_tasks import set_state
-if confirmed:
-with create_session() as session:
-altered = set_state(
-tasks=[task],
-execution_date=execution_date,
-upstream=upstream,
-downstream=downstream,
-future=future,
-past=past,
-state=state,
-commit=True,
-session=session,
-)
+with create_session() as session:
+altered = set_state(
+tasks=[task],
+execution_date=execution_date,
+upstream=upstream,
+downstream=downstream,
+future=future,
+past=past,
+state=state,
+commit=True,
+session=session,
+)
-# Clear downstream tasks that are in failed/upstream_failed
state to resume them.
-# Flush the session so that the tasks marked success are
reflected in the db.
-session.flush()
-subdag = dag.partial_subset(
-task_ids_or_regex={task_id},
-include_downstream=True,
-include_upstream=False,
-)
+# Clear downstream tasks that are in failed/upstream_failed state
to resume them.
+# Flush the session so that the tasks marked success are reflected
in the db.
+session.flush()
+subdag = dag.partial_subset(
+task_ids_or_regex={task_id},
+include_downstream=True,
+include_upstream=False,
+)
-end_date = execution_date if not future else None
-start_date = execution_date if not past else None
-
-subdag.clear(
-start_date=start_date,
-end_date=end_date,
-include_subdags=True,
-include_parentdag=True,
-only_failed=True,
-session=session,
-# Exclude the task itself from being cleared
-exclude_task_ids={task_id},
-)
+end_date = execution_date if not future else None
+start_date = execution_date if not past else None
-session.commit()
+subdag.clear(
+start_date=start_date,
+end_date=end_date,
+include_subdags=True,
+include_parentdag=True,
+only_failed=True,
+session=session,
+# Exclude the task itself from being cleared
+exclude_task_ids={task_id},
+)
-flash(f"Marked {state} on {len(altered)} task instances")
-return redirect(origin)
+session.commit()
+
+flash(f"Marked {state} on {len(altered)} task instances")
+return redirect(origin)
+
+@expose('/confirm', methods=['GET'])
+@auth.has_access(
+[
+(permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+(permissions.ACTION_CAN_EDIT, permissions.RESOURCE_TASK_INSTANCE),
+]
+)
+@action_logging
+def confirm(self):
+"""Show confirmation page for marking tasks as success or failed."""
+args = request.args
+dag_id = args.get('dag_id')
+task_id = args.get('task_id')
+execution_date = args.get('execution_date')
+state = args.get('state')
+
+upstream = to_boolean(args.get('failed_upstream'))
Review comment:
IMO it's safe to assume that `None` is `False`. Just saves a lot of code
repetition & also safeguard against handling an edge case for the future.
--
This is an automated message from the 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] uranusjr commented on a change in pull request #16233: Fix TI success/failure links
uranusjr commented on a change in pull request #16233:
URL: https://github.com/apache/airflow/pull/16233#discussion_r649406401
##
File path: airflow/www/views.py
##
@@ -1803,54 +1804,104 @@ def _mark_task_instance_state( # pylint:
disable=too-many-arguments
latest_execution_date = dag.get_latest_execution_date()
if not latest_execution_date:
-flash(f"Cannot make {state}, seem that dag {dag_id} has never
run", "error")
+flash(f"Cannot mark tasks as {state}, seem that dag {dag_id} has
never run", "error")
return redirect(origin)
execution_date = timezone.parse(execution_date)
from airflow.api.common.experimental.mark_tasks import set_state
-if confirmed:
-with create_session() as session:
-altered = set_state(
-tasks=[task],
-execution_date=execution_date,
-upstream=upstream,
-downstream=downstream,
-future=future,
-past=past,
-state=state,
-commit=True,
-session=session,
-)
+with create_session() as session:
+altered = set_state(
+tasks=[task],
+execution_date=execution_date,
+upstream=upstream,
+downstream=downstream,
+future=future,
+past=past,
+state=state,
+commit=True,
+session=session,
+)
-# Clear downstream tasks that are in failed/upstream_failed
state to resume them.
-# Flush the session so that the tasks marked success are
reflected in the db.
-session.flush()
-subdag = dag.partial_subset(
-task_ids_or_regex={task_id},
-include_downstream=True,
-include_upstream=False,
-)
+# Clear downstream tasks that are in failed/upstream_failed state
to resume them.
+# Flush the session so that the tasks marked success are reflected
in the db.
+session.flush()
+subdag = dag.partial_subset(
+task_ids_or_regex={task_id},
+include_downstream=True,
+include_upstream=False,
+)
-end_date = execution_date if not future else None
-start_date = execution_date if not past else None
-
-subdag.clear(
-start_date=start_date,
-end_date=end_date,
-include_subdags=True,
-include_parentdag=True,
-only_failed=True,
-session=session,
-# Exclude the task itself from being cleared
-exclude_task_ids={task_id},
-)
+end_date = execution_date if not future else None
+start_date = execution_date if not past else None
-session.commit()
+subdag.clear(
+start_date=start_date,
+end_date=end_date,
+include_subdags=True,
+include_parentdag=True,
+only_failed=True,
+session=session,
+# Exclude the task itself from being cleared
+exclude_task_ids={task_id},
+)
-flash(f"Marked {state} on {len(altered)} task instances")
-return redirect(origin)
+session.commit()
+
+flash(f"Marked {state} on {len(altered)} task instances")
+return redirect(origin)
+
+@expose('/confirm', methods=['GET'])
+@auth.has_access(
+[
+(permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+(permissions.ACTION_CAN_EDIT, permissions.RESOURCE_TASK_INSTANCE),
+]
+)
+@action_logging
+def confirm(self):
+"""Show confirmation page for marking tasks as success or failed."""
+args = request.args
+dag_id = args.get('dag_id')
+task_id = args.get('task_id')
+execution_date = args.get('execution_date')
+state = args.get('state')
+
+upstream = to_boolean(args.get('failed_upstream'))
Review comment:
I would avoid changing `to_boolean` and do `args.get('failed_upstream',
'')` 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] ashb merged pull request #14967: Fix too specific parsing of `False` in LegacyUIDeprecated
ashb merged pull request #14967: URL: https://github.com/apache/airflow/pull/14967 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[airflow] branch v1-10-stable updated: Fix too specific parsing of `False` in LegacyUIDeprecated (#14967)
This is an automated email from the ASF dual-hosted git repository.
ash pushed a commit to branch v1-10-stable
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/v1-10-stable by this push:
new 4a27689 Fix too specific parsing of `False` in LegacyUIDeprecated
(#14967)
4a27689 is described below
commit 4a276897169bd977f136983f929b59baac82c628
Author: Marian Cepok
AuthorDate: Thu Jun 10 19:49:45 2021 +0200
Fix too specific parsing of `False` in LegacyUIDeprecated (#14967)
---
airflow/upgrade/rules/legacy_ui_deprecated.py| 4 ++--
tests/upgrade/rules/test_legacy_ui_deprecated.py | 11 +++
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/airflow/upgrade/rules/legacy_ui_deprecated.py
b/airflow/upgrade/rules/legacy_ui_deprecated.py
index 9570af7..07617a8 100644
--- a/airflow/upgrade/rules/legacy_ui_deprecated.py
+++ b/airflow/upgrade/rules/legacy_ui_deprecated.py
@@ -28,8 +28,8 @@ class LegacyUIDeprecated(BaseRule):
def check(self):
if conf.has_option("webserver", "rbac"):
-rbac = conf.get("webserver", "rbac")
-if rbac == "false":
+rbac = conf.get("webserver", "rbac").strip().lower()
+if rbac in ("f", "false", "0"):
return (
"rbac in airflow.cfg must be explicitly set empty as"
" RBAC mechanism is enabled by default."
diff --git a/tests/upgrade/rules/test_legacy_ui_deprecated.py
b/tests/upgrade/rules/test_legacy_ui_deprecated.py
index adbbe8f..1aea7ef 100644
--- a/tests/upgrade/rules/test_legacy_ui_deprecated.py
+++ b/tests/upgrade/rules/test_legacy_ui_deprecated.py
@@ -15,14 +15,15 @@
# specific language governing permissions and limitations
# under the License.
from unittest import TestCase
+from unittest.mock import patch
from airflow.upgrade.rules.legacy_ui_deprecated import LegacyUIDeprecated
from tests.test_utils.config import conf_vars
class TestLegacyUIDeprecated(TestCase):
-@conf_vars({("webserver", "rbac"): "false"})
-def test_invalid_check(self):
+@patch('airflow.configuration.conf.get')
+def test_invalid_check(self, conf_get):
rule = LegacyUIDeprecated()
assert isinstance(rule.description, str)
@@ -32,8 +33,10 @@ class TestLegacyUIDeprecated(TestCase):
"rbac in airflow.cfg must be explicitly set empty as"
" RBAC mechanism is enabled by default."
)
-response = rule.check()
-assert response == msg
+for false_value in ("False", "false", "f", "0"):
+conf_get.return_value = false_value
+response = rule.check()
+assert response == msg
@conf_vars({("webserver", "rbac"): ""})
def test_valid_check(self):
[GitHub] [airflow] ashb commented on pull request #12925: [Loki log handler] - Integration with Grafana Loki
ashb commented on pull request #12925: URL: https://github.com/apache/airflow/pull/12925#issuecomment-858833643 For instance I wonder if you can use something like the JSON output format that ElasticSearch uses, and then combine it with https://grafana.com/docs/loki/latest/logql/#parser-expression to use date-range to limit down the logs returned, and then a pipeline like that to make sure we only get the right logs back. -- This is an automated message from the 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] bbovenzi commented on pull request #16141: Add a calendar field to choose the execution date of the DAG when triggering it
bbovenzi commented on pull request #16141: URL: https://github.com/apache/airflow/pull/16141#issuecomment-858833624 @ashb or @ryanahamilton Can we restart the CI 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] dimon222 edited a comment on pull request #16358: Only create dagruns when max_active_runs is not reached
dimon222 edited a comment on pull request #16358: URL: https://github.com/apache/airflow/pull/16358#issuecomment-858831825 Does this PR literally restrict from staggering new dagruns of there's already running one? -- This is an automated message from the 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 #16345: Correctly handle None returns from Query.scalar()
ashb commented on a change in pull request #16345:
URL: https://github.com/apache/airflow/pull/16345#discussion_r649397260
##
File path: airflow/jobs/scheduler_job.py
##
@@ -1748,6 +1748,8 @@ def _schedule_dag_run(
def _verify_integrity_if_dag_changed(self, dag_run: DagRun, session=None):
"""Only run DagRun.verify integrity if Serialized DAG has changed
since it is slow"""
latest_version =
SerializedDagModel.get_latest_version_hash(dag_run.dag_id, session=session)
+if latest_version is None:
+raise AirflowException(f"Serialized DAG not found for DAG run
{dag_run.id}.")
Review comment:
What does raising an exception here do to the scheduler loop?
--
This is an automated message from the 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] dimon222 commented on pull request #16358: Only create dagruns when max_active_runs is not reached
dimon222 commented on pull request #16358: URL: https://github.com/apache/airflow/pull/16358#issuecomment-858831825 Does this PR literally restrict from queuing new dagruns of there's already running one? -- This is an automated message from the 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 #12925: [Loki log handler] - Integration with Grafana Loki
ashb commented on pull request #12925: URL: https://github.com/apache/airflow/pull/12925#issuecomment-858830114 Oh. try_number is going to confuse things too; The the start/end dates in the TI will be for the latest try only, so you'll need to look elsewhere (TaskFail table I think) for those dates. -- This is an automated message from the 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 #12925: [Loki log handler] - Integration with Grafana Loki
ashb commented on pull request #12925: URL: https://github.com/apache/airflow/pull/12925#issuecomment-858828896 You can have multiple tasks for different dag runs running concurrently, so filtering for logs between start and end date would not be a complete solution -- you would still get logs from TIs interleaved in one. -- This is an automated message from the 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] sshah90 commented on pull request #12925: [Loki log handler] - Integration with Grafana Loki
sshah90 commented on pull request #12925: URL: https://github.com/apache/airflow/pull/12925#issuecomment-858826335 Hi @ashb, > Not including execution_date in the task labels is incorrect -- the execution date is needed to uniquely identify the task instance I agree, in some instances, you can't uniquely identify the task. However, adding this label (execution_date) would exponentially increase the cardinality in Loki to the point that it would be inoperable. This was discussed at great length with the Loki team and therefore, it was intentionally left out. This is also documented [here](https://grafana.com/docs/loki/latest/best-practices/#use-dynamic-labels-sparingly). > by not including this it means that logs from the task on two separate days would get combined. This is true however we use `start_time` to filter the log data so that it's not combined when viewing the task logs in the UI. -- This is an automated message from the 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] anitakar opened a new pull request #16376: Set num of retries for google cloud connection to 3.
anitakar opened a new pull request #16376: URL: https://github.com/apache/airflow/pull/16376 Can be useful in case of a connection being kept alive for a very long time, for example because underneth it waits for a dataflow job. related: #1397 -- This is an automated message from the 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] uranusjr commented on a change in pull request #16110: Added windows extensions
uranusjr commented on a change in pull request #16110:
URL: https://github.com/apache/airflow/pull/16110#discussion_r649381846
##
File path: airflow/utils/process_utils.py
##
@@ -79,7 +86,7 @@ def signal_procs(sig):
else:
raise
-if pgid == os.getpgid(0):
+if IS_WINDOWS and pgid == os.getpid() or not IS_WINDOWS and pgid ==
os.getpgid(0):
Review comment:
I think this needs some parentheses? At least for readability.
##
File path: setup.cfg
##
@@ -156,6 +156,9 @@ install_requires =
typing-extensions>=3.7.4;python_version<"3.8"
unicodecsv>=0.14.1
werkzeug~=1.0, >=1.0.1
+# needed for generating virtual python environments when running tasks
+virtualenv>=20.4.3
+psycopg2>=2.8.6
Review comment:
Why is psycopg needed?
##
File path: airflow/utils/process_utils.py
##
@@ -155,11 +162,13 @@ def execute_interactive(cmd: List[str], **kwargs):
"""
log.info("Executing cmd: %s", " ".join(shlex.quote(c) for c in cmd))
-old_tty = termios.tcgetattr(sys.stdin)
-tty.setraw(sys.stdin.fileno())
+if not IS_WINDOWS:
+old_tty = termios.tcgetattr(sys.stdin)
+tty.setraw(sys.stdin.fileno())
+
+# open pseudo-terminal to interact with subprocess
+master_fd, slave_fd = pty.openpty()
Review comment:
I don’t think the function works after the patch (at least `slave_fd`
would become undefined). Since this function is only used to run database
commands, maybe we should mark this function as POSIX-only, and implement a
separate function for Windows. (Would it be enough to simply brige
stdin/out/err to the subprocess?)
##
File path: airflow/utils/timeout.py
##
@@ -31,20 +34,34 @@ def __init__(self, seconds=1, error_message='Timeout'):
self.seconds = seconds
self.error_message = error_message + ', PID: ' + str(os.getpid())
-def handle_timeout(self, signum, frame): # pylint: disable=unused-argument
+def handle_timeout(self, *args): # pylint: disable=unused-argument
"""Logs information and raises AirflowTaskTimeout."""
self.log.error("Process timed out, PID: %s", str(os.getpid()))
raise AirflowTaskTimeout(self.error_message)
def __enter__(self):
try:
-signal.signal(signal.SIGALRM, self.handle_timeout)
-signal.setitimer(signal.ITIMER_REAL, self.seconds)
+if IS_WINDOWS:
+if hasattr(self, TIMER_THREAD_ATTR) and getattr(self,
TIMER_THREAD_ATTR) is not None:
+getattr(self, TIMER_THREAD_ATTR).cancel()
+timer = Timer(self.seconds, self.handle_timeout)
+setattr(self, TIMER_THREAD_ATTR, timer)
+timer.start()
Review comment:
The exception message below (about hte current context) doesn’t apply to
the thread-based implementation, so we should move the `if IS_WINDOWS:` block
out of `try:`.
##
File path: airflow/utils/timeout.py
##
@@ -31,20 +34,34 @@ def __init__(self, seconds=1, error_message='Timeout'):
self.seconds = seconds
self.error_message = error_message + ', PID: ' + str(os.getpid())
-def handle_timeout(self, signum, frame): # pylint: disable=unused-argument
+def handle_timeout(self, *args): # pylint: disable=unused-argument
"""Logs information and raises AirflowTaskTimeout."""
self.log.error("Process timed out, PID: %s", str(os.getpid()))
raise AirflowTaskTimeout(self.error_message)
def __enter__(self):
try:
-signal.signal(signal.SIGALRM, self.handle_timeout)
-signal.setitimer(signal.ITIMER_REAL, self.seconds)
+if IS_WINDOWS:
+if hasattr(self, TIMER_THREAD_ATTR) and getattr(self,
TIMER_THREAD_ATTR) is not None:
+getattr(self, TIMER_THREAD_ATTR).cancel()
+timer = Timer(self.seconds, self.handle_timeout)
+setattr(self, TIMER_THREAD_ATTR, timer)
+timer.start()
Review comment:
BTW why do we need to use `has|get|set_attr` here? Why not put them on
the instance instead, they would just be unused on POSIX. Or we can do
something like
```python
_timeout = ContextManager[None]
class _timeout_windows(_timeout):
... # Implementation for Windows.
class _timeout_posix(_timeout):
... # Implementation for POSIX.
if IS_WINDOWS:
timeout: Type[_timeout] = _timeout_windows
else:
timeout = _timeout_posix
```
--
This is an automated message from the 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] jpyen commented on pull request #16318: Fix API List DAGs inconsistent with UI/CLI
jpyen commented on pull request #16318: URL: https://github.com/apache/airflow/pull/16318#issuecomment-858817951 @kaxil Changes approved, checks passed and ready to merge. Thanks for the patience. -- This is an automated message from the 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 pull request #16343: Fix DAG run state not updated while DAG is paused
ephraimbuddy commented on pull request #16343: URL: https://github.com/apache/airflow/pull/16343#issuecomment-858810600 > > Nice but I think it may not work if the user disables mini scheduling? > > Yes, but we'll likely remove that setting in a version or two -- it was mostly an escape hatch in case it had un-forseen bugs. Should I add it as a separate check outside the mini scheduling? -- This is an automated message from the 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 main updated (6e9e562 -> 9351e2a)
This is an automated email from the ASF dual-hosted git repository. ash pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/airflow.git. from 6e9e562 Make task ID on legend have enough width and width of line chart to be 100%. (#15915) add 9351e2a Depreciate private_key_pass in SFTPHook conn extra and rename to private_key_passphrase (#14028) No new revisions were added by this update. Summary of changes: airflow/providers/sftp/CHANGELOG.rst | 6 ++ airflow/providers/sftp/hooks/sftp.py | 15 +-- 2 files changed, 19 insertions(+), 2 deletions(-)
[GitHub] [airflow] ashb commented on pull request #16343: Fix DAG run state not updated while DAG is paused
ashb commented on pull request #16343: URL: https://github.com/apache/airflow/pull/16343#issuecomment-858808709 > Nice but I think it may not work if the user disables mini scheduling? Yes, but we'll likely remove that setting in a version or two -- it was mostly an escape hatch in case it had un-forseen bugs. -- This is an automated message from the 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 merged pull request #14028: Depreciate private_key_pass extra param and rename to private_key_passphrase
ashb merged pull request #14028: URL: https://github.com/apache/airflow/pull/14028 -- This is an automated message from the 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 pull request #14028: Depreciate private_key_pass extra param and rename to private_key_passphrase
boring-cyborg[bot] commented on pull request #14028: URL: https://github.com/apache/airflow/pull/14028#issuecomment-858808056 Awesome work, congrats on your first merged pull request! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] ephraimbuddy commented on pull request #16343: Fix DAG run state not updated while DAG is paused
ephraimbuddy commented on pull request #16343: URL: https://github.com/apache/airflow/pull/16343#issuecomment-858806028 > I wonder what is more efficient: doing this periodically (for paused dags, where the state is likely to never change) or expanding on the "mini scheduler run" to do a simpler version of `dag_run.update_state()` when the task that just finished was one of the leaf tasks in the dag. Nice but I think it may not work if the user disables mini scheduling? -- This is an automated message from the 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] bbovenzi opened a new pull request #16375: Fix normalize-url vulnerability
bbovenzi opened a new pull request #16375: URL: https://github.com/apache/airflow/pull/16375 Update two packages that used a highly vulnerable version of normalize-url See https://github.com/facebook/create-react-app/issues/11054 --- **^ Add meaningful description above** Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information. In case of fundamental code 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/main/UPDATING.md). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] jhtimmins commented on pull request #16110: Added windows extensions
jhtimmins commented on pull request #16110: URL: https://github.com/apache/airflow/pull/16110#issuecomment-858803535 @ashb can you take a look to see if your requested changes have been addressed? -- This is an automated message from the 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] uranusjr commented on a change in pull request #16352: DagRun.date_interval_start and date_interval_start
uranusjr commented on a change in pull request #16352: URL: https://github.com/apache/airflow/pull/16352#discussion_r649376333 ## File path: airflow/migrations/versions/142555e44c17_add_data_interval_start_end_to_dagrun.py ## @@ -0,0 +1,100 @@ +# +# 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. + +"""Add data_interval_[start|end] to DagRun. + +Revision ID: 142555e44c17 +Revises: e9304a3141f0 +Create Date: 2021-06-09 08:28:02.089817 + +""" + +import alembic +import sqlalchemy + +from airflow.models import DagModel, DagRun Review comment: I chose to only pull out a minimal set of fields we need in the migration (a trick I sometimes do to make the migrations more readable). -- This is an automated message from the 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] jhtimmins commented on pull request #15947: Applied permissions to self._error_file
jhtimmins commented on pull request #15947: URL: https://github.com/apache/airflow/pull/15947#issuecomment-858802569 @kevinbsilva looks like these are legit test failures that need to be addressed https://github.com/apache/airflow/pull/15947/checks?check_run_id=2794600747#step:6:14114 -- This is an automated message from the 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] anitakar commented on a change in pull request #16311: Prevent running `airflow db init` migrations and setup in parallel.
anitakar commented on a change in pull request #16311:
URL: https://github.com/apache/airflow/pull/16311#discussion_r649373985
##
File path: airflow/utils/db.py
##
@@ -561,10 +571,21 @@ def create_default_connections(session=None):
)
-def initdb():
+@provide_session
+def initdb(session=None):
"""Initialize Airflow database."""
+if session.connection().dialect.name == 'postgresql':
+log.info('Acquiring lock on database')
+session.connection().execute('select PG_ADVISORY_LOCK(1);')
+
upgradedb()
+if session.connection().dialect.name == 'mysql' and
session.connection().dialect.server_version_info >= (
+5,
+6,
+):
+session.connection().execute("select GET_LOCK('db_init',1800);")
Review comment:
Done
##
File path: airflow/migrations/env.py
##
@@ -101,9 +101,6 @@ def run_migrations_online():
with context.begin_transaction():
if connection.dialect.name == 'mysql' and
connection.dialect.server_version_info >= (5, 6):
connection.execute("select GET_LOCK('alembic',1800);")
-if connection.dialect.name == 'postgresql':
-context.get_context()._ensure_version_table() # pylint:
disable=protected-access
-connection.execute("LOCK TABLE alembic_version IN ACCESS
EXCLUSIVE MODE")
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] jhtimmins commented on pull request #11652: Support for sorting DAGs in the web UI
jhtimmins commented on pull request #11652: URL: https://github.com/apache/airflow/pull/11652#issuecomment-858799643 @scrdest can you fix these merge conflicts? If so we'll target 2.2 for this release. @ashb bumping since you requested changes -- This is an automated message from the 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 pull request #16358: Only create dagruns when max_active_runs is not reached
ephraimbuddy commented on pull request #16358: URL: https://github.com/apache/airflow/pull/16358#issuecomment-858794233 > Tackling #16366 _may_ be less work, not sure, but it's certainly a better fix long term I have updated this PR and will start working on #16366. From my thinking around the queued state, this change may still be valid. Basing my PR on this for 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] ephraimbuddy commented on a change in pull request #16358: Only create dagruns when max_active_runs is not reached
ephraimbuddy commented on a change in pull request #16358: URL: https://github.com/apache/airflow/pull/16358#discussion_r649364776 ## File path: airflow/jobs/scheduler_job.py ## @@ -1593,7 +1592,9 @@ def _create_dag_runs(self, dag_models: Iterable[DagModel], session: Session) -> # we need to run self._update_dag_next_dagruns if the Dag Run already exists or if we # create a new one. This is so that in the next Scheduling loop we try to create new runs # instead of falling in a loop of Integrity Error. -if (dag.dag_id, dag_model.next_dagrun) not in active_dagruns: +if (dag.dag_id, dag_model.next_dagrun) not in active_dagruns and len( +active_dagruns +) < dag.max_active_runs: Review comment: I have corrected it. Surprised why it was working previously. Thanks -- This is an automated message from the 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] github-actions[bot] commented on pull request #14028: Depreciate private_key_pass extra param and rename to private_key_passphrase
github-actions[bot] commented on pull request #14028: URL: https://github.com/apache/airflow/pull/14028#issuecomment-858782384 The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease. -- This is an automated message from the 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
