Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Allow "/" in metrics validator #42934

Merged
merged 3 commits into from
Nov 30, 2024
Merged

Conversation

awdavidson
Copy link
Contributor

Changes Proposed in this PR

  • Allow '/' in metrics validator, this resolves issues such as
airflow.exceptions.InvalidStatsNameException: The stat name (dag_processing.processes,file_path=/mnt/c/Users/user/Documents/GitHub/airflow-dir/test_dag.py,action=finish) has to be composed of ASCII alphabets, numbers, or the underscore, dot, or dash characters.
[2023-04-18T12:21:39.738-0400] {stats.py:245} ERROR - Invalid stat name: dag_processing.processes,file_path=/mnt/c/Users/user/Documents/GitHub/airflow-dir/test_dag.py,action=start.
Traceback (most recent call last):
File "/mnt/c/Users/user/Documents/GitHub/airflow-dir/venv/lib/python3.9/site-packages/airflow/stats.py", line 242, in wrapper
stat = handler_stat_name_func(stat)
File "/mnt/c/Users/user/Documents/GitHub/airflow-dir/venv/lib/python3.9/site-packages/airflow/stats.py", line 210, in stat_name_default_handler
raise InvalidStatsNameException(
airflow.exceptions.InvalidStatsNameException: The stat name (dag_processing.processes,file_path=/mnt/c/Users/user/Documents/GitHub/airflow-dir/test_dag.py,action=start) has to be composed of ASCII alphabets, numbers, or the underscore, dot, or dash characters.
[2023-04-18T12:21:51.375-0400] {stats.py:245} ERROR - Invalid stat name: dag_processing.processes,file_path=/mnt/c/Users/user/Documents/GitHub/airflow-dir/test_dag.py,action=finish.

closes: #30716


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@potiuk potiuk added this to the Airflow 2.10.3 milestone Oct 14, 2024
@potiuk
Copy link
Member

potiuk commented Oct 14, 2024

Two asks:

  1. Can you please add a unit test for that
  2. This look like a bug-fix - can you please cherry-pick (with -x) this change after I merge it to v2-10-test targeted-PR ?

@potiuk
Copy link
Member

potiuk commented Nov 28, 2024

Some static checks to fix. I recommend installing pre-commit - it will solve problems for you

@potiuk
Copy link
Member

potiuk commented Nov 30, 2024

Nice!

@potiuk potiuk merged commit 14b32ea into apache:main Nov 30, 2024
49 checks passed
@shalberd
Copy link

shalberd commented Nov 30, 2024

Very nice indeed, thank you so very much. @potiuk will this be backported and included in 2.10.3 or only in a future Airflow v3?

@potiuk
Copy link
Member

potiuk commented Nov 30, 2024

Ah ... It should be .. but I forgot to put the label on it... So let me try to cherry-pick it now.

potiuk pushed a commit to potiuk/airflow that referenced this pull request Nov 30, 2024
* Allow "/" to avoid ERROR - Invalid stat name: dag_processing.processes,file_path=/mnt/c

* Add UT

* Reformat
(cherry picked from commit 14b32ea)

Co-authored-by: awdavidson <54780428+awdavidson@users.noreply.github.com>
@potiuk
Copy link
Member

potiuk commented Nov 30, 2024

#44515

potiuk added a commit that referenced this pull request Nov 30, 2024
* Allow "/" to avoid ERROR - Invalid stat name: dag_processing.processes,file_path=/mnt/c

* Add UT

* Reformat
(cherry picked from commit 14b32ea)

Co-authored-by: awdavidson <54780428+awdavidson@users.noreply.github.com>
utkarsharma2 pushed a commit that referenced this pull request Dec 4, 2024
* Allow "/" to avoid ERROR - Invalid stat name: dag_processing.processes,file_path=/mnt/c

* Add UT

* Reformat
(cherry picked from commit 14b32ea)

Co-authored-by: awdavidson <54780428+awdavidson@users.noreply.github.com>
utkarsharma2 pushed a commit that referenced this pull request Dec 9, 2024
* Allow "/" to avoid ERROR - Invalid stat name: dag_processing.processes,file_path=/mnt/c

* Add UT

* Reformat
(cherry picked from commit 14b32ea)

Co-authored-by: awdavidson <54780428+awdavidson@users.noreply.github.com>
LefterisXefteris pushed a commit to LefterisXefteris/airflow that referenced this pull request Jan 5, 2025
* Allow "/" to avoid ERROR - Invalid stat name: dag_processing.processes,file_path=/mnt/c

* Add UT

* Reformat
got686-yandex pushed a commit to got686-yandex/airflow that referenced this pull request Jan 30, 2025
* Allow "/" to avoid ERROR - Invalid stat name: dag_processing.processes,file_path=/mnt/c

* Add UT

* Reformat
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DagFileProcessor produces invalid metric tags
5 participants