You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "sidshas03 (via GitHub)" <gi...@apache.org> on 2025/09/17 04:17:30 UTC
[PR] Validate DAG tag length with clear error; document 100-char per-tag limit [airflow]
sidshas03 opened a new pull request, #55750:
URL: https://github.com/apache/airflow/pull/55750
### Problem
Issue #55693 reported confusion about DAG tag length limits. Users thought there was a "combined length of all tags" limit, but the actual database constraint is **100 characters per individual tag** (`dag_tag.name VARCHAR(100)`). The error surfaced late and wasn't clearly documented.
### Changes
- Add early tag-length validation in DAG creation paths (SDK DAG)
- Raise `AirflowException` with clear error messages naming the offending tag and its actual length
- Add documentation clarifying the per-tag limit (not combined length)
- Preview long tags in error messages for readability
### Why safe
- Aligns with existing database schema (`dag_tag.name VARCHAR(100)`)
- No migrations or behavior changes beyond clearer, earlier validation
- Clarifies the actual constraint vs. user misconception
### Tests
- Tag of exactly 100 chars → allowed
- Tag of 101 chars → raises `AirflowException` (message includes actual length and limit)
- Multiple tags with one too long → raises; message identifies the problematic tag via preview
### Docs
- Added "Tag limitations" subsection clarifying per-tag 100-character limit
Resolves #55693
--
This is an automated message from the 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] Validate DAG tag length with clear error; document 100-char per-tag limit [airflow]
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on code in PR #55750:
URL: https://github.com/apache/airflow/pull/55750#discussion_r2403161979
##########
docs/apache-airflow/core-concepts/dags.rst:
##########
@@ -0,0 +1,5 @@
+# DAGs
+
+## Tag limitations
+
+Each individual tag must be 100 characters or less. Tags longer than 100 characters will cause a validation error during DAG parsing with a clear error message indicating the offending tag and its length.
Review Comment:
i'm not even sure we really need to document this since users will find out when they hit the error at parse tiem
--
This is an automated message from the 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] Validate DAG tag length with clear error; document 100-char per-tag limit [airflow]
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on code in PR #55750:
URL: https://github.com/apache/airflow/pull/55750#discussion_r2403188836
##########
tests/models/test_dag_tags.py:
##########
@@ -0,0 +1,78 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
here too it looks like you are adding a new file---- you should not need to add a new file you must be putting it in wrong place it seems
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Validate DAG tag length with clear error; document 100-char per-tag limit [airflow]
Posted by "HsiuChuanHsu (via GitHub)" <gi...@apache.org>.
HsiuChuanHsu commented on PR #55750:
URL: https://github.com/apache/airflow/pull/55750#issuecomment-3306575277
Looks good.
> I'm not a committer to the project, but these changes look good to me. One thing; I have seen a conversation about not using the `AirflowException` anymore.
A quick suggestion:
It is true that we're all pretty much in agreement to stop using AirflowException directly.
https://lists.apache.org/thread/t8bnhyqy77kq4fk7fj3fmjd5wo9kv6w0
Maybe we should go ahead for this situation.
--
This is an automated message from the 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] Validate DAG tag length with clear error; document 100-char per-tag limit [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #55750:
URL: https://github.com/apache/airflow/pull/55750#issuecomment-3826681564
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Validate DAG tag length with clear error; document 100-char per-tag limit [airflow]
Posted by "m1racoli (via GitHub)" <gi...@apache.org>.
m1racoli commented on PR #55750:
URL: https://github.com/apache/airflow/pull/55750#issuecomment-3572899420
@sidshas03 just little poke, in case you have plans to continue on this PR. :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Validate DAG tag length with clear error; document 100-char per-tag limit [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on code in PR #55750:
URL: https://github.com/apache/airflow/pull/55750#discussion_r2403190449
##########
task-sdk/src/airflow/sdk/definitions/dag.py:
##########
@@ -557,9 +557,17 @@ def _validate_catchup(self, _, catchup: bool):
raise ValueError("start_date is required when catchup=True")
@tags.validator
- def _validate_tags(self, _, tags: Collection[str]):
- if tags and any(len(tag) > TAG_MAX_LEN for tag in tags):
- raise ValueError(f"tag cannot be longer than {TAG_MAX_LEN} characters")
+ def _validate_tags(self, _, tags: Collection[str]) -> None:
+ if tags:
+ for tag in tags:
+ if len(tag) > TAG_MAX_LEN:
+ from airflow.exceptions import AirflowException
+ # Trim very long tag previews to keep error messages manageable
+ tag_preview = tag[:30] + "..." if len(tag) > 30 else tag
+ raise AirflowException(
Review Comment:
We don't want to introduce or use new cases of AirflowException `ValueError` is more appopriate:
> Raised when an operation or function receives an argument that has the right type but an inappropriate value
```suggestion
raise ValueError(
```
--
This is an automated message from the 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] Validate DAG tag length with clear error; document 100-char per-tag limit [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on code in PR #55750:
URL: https://github.com/apache/airflow/pull/55750#discussion_r2403187655
##########
tests/models/test_dag_tags.py:
##########
Review Comment:
This should now live in an existing file under `task-sdk/tests/` to match the location where you made the 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Validate DAG tag length with clear error; document 100-char per-tag limit [airflow]
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on code in PR #55750:
URL: https://github.com/apache/airflow/pull/55750#discussion_r2403161132
##########
docs/apache-airflow/core-concepts/dags.rst:
##########
@@ -0,0 +1,5 @@
+# DAGs
Review Comment:
doesn't this section already exist here? `airflow-core/docs/core-concepts/dags.rst`?
--
This is an automated message from the 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] Validate DAG tag length with clear error; document 100-char per-tag limit [airflow]
Posted by "sidshas03 (via GitHub)" <gi...@apache.org>.
sidshas03 commented on PR #55750:
URL: https://github.com/apache/airflow/pull/55750#issuecomment-3303759606
> I'm not a committer to the project, but these changes look good to me. One thing; I have seen a conversation about not using the `AirflowException` anymore.
Hey @jroachgolf84,
Good call on AirflowException. To align with SDK guidance, I’ll switch the validator to raise a standard ValueError (invalid user input) instead of AirflowException, keep the same clear message (with the trimmed tag preview + actual length), and update the tests accordingly. I’ll also ensure the docs describe a “clear error during DAG parse” without naming a specific exception class.
If there’s a project-preferred domain exception for SDK validators, I’m happy to adopt it, but I’ll proceed with ValueError for consistency and can adjust if maintainers suggest otherwise.
--
This is an automated message from the 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] Validate DAG tag length with clear error; document 100-char per-tag limit [airflow]
Posted by "sidshas03 (via GitHub)" <gi...@apache.org>.
sidshas03 commented on PR #55750:
URL: https://github.com/apache/airflow/pull/55750#issuecomment-3366874453
I'm really sorry for the late reply. I have Switched the tag-length validator to **`ValueError`**, removed the `AirflowException` import, and updated tests to expect `ValueError`. Message unchanged (trimmed tag preview + actual length + limit). Docs remain neutral.
--
This is an automated message from the 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] Validate DAG tag length with clear error; document 100-char per-tag limit [airflow]
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on code in PR #55750:
URL: https://github.com/apache/airflow/pull/55750#discussion_r2403189345
##########
tests/models/test_dag_tags.py:
##########
@@ -0,0 +1,78 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
(there is already a test_dag_tags file somewhere)
--
This is an automated message from the 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] Validate DAG tag length with clear error; document 100-char per-tag limit [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #55750:
URL: https://github.com/apache/airflow/pull/55750#issuecomment-3565072937
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Validate DAG tag length with clear error; document 100-char per-tag limit [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #55750: Validate DAG tag length with clear error; document 100-char per-tag limit
URL: https://github.com/apache/airflow/pull/55750
--
This is an automated message from the 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] Validate DAG tag length with clear error; document 100-char per-tag limit [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on code in PR #55750:
URL: https://github.com/apache/airflow/pull/55750#discussion_r2403185426
##########
task-sdk/src/airflow/sdk/definitions/dag.py:
##########
@@ -557,9 +557,17 @@ def _validate_catchup(self, _, catchup: bool):
raise ValueError("start_date is required when catchup=True")
@tags.validator
- def _validate_tags(self, _, tags: Collection[str]):
- if tags and any(len(tag) > TAG_MAX_LEN for tag in tags):
- raise ValueError(f"tag cannot be longer than {TAG_MAX_LEN} characters")
+ def _validate_tags(self, _, tags: Collection[str]) -> None:
+ if tags:
Review Comment:
The `if tags:` is pointless here. We didn't have it before, so we can remove it (as a general rule, the less indented the code, the easier it is to read later.
--
This is an automated message from the 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] Validate DAG tag length with clear error; document 100-char per-tag limit [airflow]
Posted by "m1racoli (via GitHub)" <gi...@apache.org>.
m1racoli commented on PR #55750:
URL: https://github.com/apache/airflow/pull/55750#issuecomment-3366787884
Hey @sidshas03, thank you for your PR. :) Do you know when you might be able to address the suggestions?
ValueError is a good choice btw. :)
--
This is an automated message from the 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] Validate DAG tag length with clear error; document 100-char per-tag limit [airflow]
Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on code in PR #55750:
URL: https://github.com/apache/airflow/pull/55750#discussion_r2403222420
##########
task-sdk/tests/task_sdk/definitions/test_dag_tags.py:
##########
@@ -0,0 +1,78 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
oh -- yeah listen to @ashb's 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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org