You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/06/09 03:45:28 UTC
[GitHub] [airflow] uranusjr opened a new pull request #16345: Correctly handle None returns from Query.scalar()
uranusjr opened a new pull request #16345:
URL: https://github.com/apache/airflow/pull/16345
Toward #8171, fix #16328.
`None` is possible when the query does not return a row, according to SQLAlchemy documentation. We can handle them to provide better errors in unexpected situations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil merged pull request #16345: Correctly handle None returns from Query.scalar()
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #16345:
URL: https://github.com/apache/airflow/pull/16345
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #16345: Correctly handle None returns from Query.scalar()
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [airflow] uranusjr closed pull request #16345: Correctly handle None returns from Query.scalar()
Posted by GitBox <gi...@apache.org>.
uranusjr closed pull request #16345:
URL: https://github.com/apache/airflow/pull/16345
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #16345: Correctly handle None returns from Query.scalar()
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #16345: Correctly handle None returns from Query.scalar()
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #16345: Correctly handle None returns from Query.scalar()
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16345:
URL: https://github.com/apache/airflow/pull/16345#issuecomment-857619242
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:
users@infra.apache.org