You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "tatiana (via GitHub)" <gi...@apache.org> on 2024/05/16 14:22:34 UTC
[PR] Change dataset validation to raise warning [airflow]
tatiana opened a new pull request, #39670:
URL: https://github.com/apache/airflow/pull/39670
<!--
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.
-->
Closes: #39486
Valid DAGs that worked in Airflow 2.8.x and had tasks with outlets with specific URIs, such as `Dataset("postgres://postgres:5432/postgres.dbt.stg_customers")`, stopped working in Airflow 2.9.0 & Airflow 2.9.1, after #37005 was merged.
This was a breaking change in an Airflow minor version. We should avoid this.
Airflow < 3.0 should raise a warning, and from Airflow 3.0, we can make errors by default. We can have a feature flag to allow users who want to see this in advance to enable errors in Airflow 2. x, but this should not be the default behaviour.
The DAGs should continue working on Airflow 2.x minor/micro releases without errors (unless the user opts in via configuration).
By running the following DAG with `apache-airflow==2.9.1` and `apache-airflow-providers-postgres==5.11.0`, as an example:
```
from datetime import datetime
from airflow import DAG
from airflow.datasets import Dataset
from airflow.operators.empty import EmptyOperator
with DAG(dag_id='empty_operator_example', start_date=datetime(2022, 1, 1), schedule_interval=None) as dag:
task1 = EmptyOperator(
task_id='empty_task1',
dag=dag,
outlets=[Dataset("postgres://postgres:5432/postgres.dbt.stg_customers")]
)
task2 = EmptyOperator(
task_id='empty_task2',
dag=dag
)
task1 >> task2
```
Causes to the exception:
```
Broken DAG: [/usr/local/airflow/dags/example_issue.py]
Traceback (most recent call last):
File "/usr/local/lib/python3.11/site-packages/airflow/datasets/__init__.py", line 81, in _sanitize_uri
parsed = normalizer(parsed)
^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/airflow/providers/postgres/datasets/postgres.py", line 34, in sanitize_uri
raise ValueError("URI format postgres:// must contain database, schema, and table names")
ValueError: URI format postgres:// must contain database, schema, and table names
```
This PR introduces the following:
1. A boolean configuration within `[core],` named `strict_dataset_uri_validation,` which should be `False` by default.
2. When this configuration is `False,` Airflow should raise a warning saying:
```
From Airflow 3, Airflow will be more strict with Dataset URIs, and the URI xx will no longer be valid. Please, follow the expected standard as documented in XX.
```
3. If this configuration is `True,` Airflow should raise the exception, as it does now in Airflow 2.9.0 and 2.9.1
4. From Airflow 3.0, we change this configuration to be True by default.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Change dataset URI validation to raise warning instead of error in Airflow 2.9 [airflow]
Posted by "pankajkoti (via GitHub)" <gi...@apache.org>.
pankajkoti commented on PR #39670:
URL: https://github.com/apache/airflow/pull/39670#issuecomment-2116872274
Since we have multiple approvals on this, I'm merging this. However, please drop a comment here in case someone has comments and we can address those in a subsequent PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Change dataset URI validation to raise warning instead of error in Airflow 2.9 [airflow]
Posted by "pankajkoti (via GitHub)" <gi...@apache.org>.
pankajkoti merged PR #39670:
URL: https://github.com/apache/airflow/pull/39670
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Change dataset URI validation to raise warning instead of error in Airflow 2.9 [airflow]
Posted by "pankajkoti (via GitHub)" <gi...@apache.org>.
pankajkoti commented on code in PR #39670:
URL: https://github.com/apache/airflow/pull/39670#discussion_r1603803738
##########
airflow/datasets/__init__.py:
##########
@@ -87,7 +90,18 @@ def _sanitize_uri(uri: str) -> str:
fragment="", # Ignore any fragments.
)
if (normalizer := _get_uri_normalizer(normalized_scheme)) is not None:
- parsed = normalizer(parsed)
+ try:
+ parsed = normalizer(parsed)
+ except ValueError as exception:
+ if conf.getboolean("core", "strict_dataset_uri_validation"):
Review Comment:
```suggestion
if conf.getboolean("core", "strict_dataset_uri_validation", fallback=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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org