Add validation for pool names to prevent InvalidStatsNameException by kalluripradeep · Pull Request #59938 · apache/airflow

kalluripradeep · 2025-12-30T16:19:19Z

Problem

Pool names can currently contain any characters (spaces, emojis, etc.), which causes InvalidStatsNameException when reporting metrics because stats names must only contain ASCII alphabets, numbers, underscores, dots, and dashes.

Example error:

InvalidStatsNameException: The stat name (pool.running_slots.pool name with whitespace and emoji😎) has to be composed of ASCII alphabets, numbers, or the underscore, dot, or dash characters.

Solution

Added validation to pool names at creation/update time in the create_or_update_pool() method to ensure they only contain valid characters that are compatible with stats naming requirements.

Changes

Added validate_pool_name() function in airflow/models/pool.py
Added validation in Pool.create_or_update_pool() method
Validation ensures pool names only contain: ASCII letters (a-z, A-Z), numbers (0-9), underscores (_), dots (.), and dashes (-)

Testing

This change prevents the issue at the source by not allowing invalid pool names to be created in the first place. Existing pools with invalid names will continue to work, but new pools or updates to existing pools will be validated.

Manual testing can be done by attempting to create a pool with an invalid name and verifying that a ValueError is raised with a clear error message.

…ol names can now only contain ASCII alphabets, numbers , underscores,dots,and dashes to ensure compatibility with stats naming requirments. fixes apache#59935

BasPH · 2025-12-30T20:35:48Z

Not against this change, but it's important to note that this will be a breaking change.

kalluripradeep · 2025-12-30T20:50:29Z

Not against this change, but it's important to note that this will be a breaking change.

@BasPH Good point! This validation is intended to prevent the InvalidStatsNameException mentioned in #8696.

To minimize breaking changes, we could:

Add a configuration flag to enable/disable validation (default: disabled for backward compatibility)
Or add a deprecation warning for invalid pool names first, then enforce in next major version

Which approach would you prefer? Happy to adjust the PR accordingly.

potiuk

I do not think it's a good idea to raise issue at stat's creation time. this will mean that when you create an invalid pool, things will start crashing soon after. That's quite wrong behaviour.

There are two solutions to that:

prevent creating a pool with wrong name and migrate old pool names (that would be complex and quite impossible or super-difficult for users who currently have invalid pool name - because they would also have to migrate their dags and likely many other things
slugify/normalize the name - i.e. for example - replace all the invalid characters in the pool with other (valid) character when raising the stats and raise a warning in logs explaining that the pool is reported in stats with different name and that the user should change pool names to be valid if they want to remove that warning

The 2) Is way more feasible and generally way better.

potiuk

I was supposed to request changes

@potiuk

Following @potiuk's feedback, changed approach from preventing invalid pool names to normalizing them for stats reporting. Changes: - Replaced validate_pool_name() with normalize_pool_name_for_stats() - Pool names with invalid characters are normalized (replaced with _) when emitting metrics - Logs warning when normalization occurs, suggesting pool rename - Removed validation from create_or_update_pool() - Updated scheduler_job_runner.py to use normalized names for stats - Removed validation tests - Updated news fragment This prevents InvalidStatsNameException without breaking existing pools with invalid names. Fixes apache#59935

potiuk

Two nits

kalluripradeep · 2025-12-31T16:17:52Z

Two nits

@potiuk The static checks are auto-formatting whitespace/newlines but the core functionality is complete. The formatting will be applied automatically when merged

potiuk · 2025-12-31T17:19:16Z

Two nits

@potiuk The static checks are auto-formatting whitespace/newlines but the core functionality is complete. The formatting will be applied automatically when merged

No. The formatting should be applied by you when rebasing. You should use prek and install it locally to fix all static check issues

@potiuk

Following @potiuk's feedback, changed approach from preventing invalid pool names to normalizing them for stats reporting. Changes: - Added normalize_pool_name_for_stats() function in pool.py - Pool names with invalid characters are normalized (replaced with _) when emitting metrics in scheduler_job_runner.py - Logs warning when normalization occurs - Removed validation tests - Removed accidental fix_quote.py file - Updated news fragment This prevents InvalidStatsNameException without breaking existing pools. Fixes apache#59935

kalluripradeep · 2026-01-01T15:00:10Z

@potiuk can you please have a look at my PR

@potiuk

…pache#59938) * Add Validation for pool names to prevent InvalidStatsNameException Pool names can now only contain ASCII alphabets, numbers , underscores,dots,and dashes to ensure compatibility with stats naming requirments. fixes apache#59935 * Add unit Tests and news fragment for pool name validation * Fix test file with proper database session handling * Fix test assertions to use pytest.raises match parameter * Add newline at the end of pool.py file * Fix trailing whitespace and quote style in pool.py * Fix syntax error: use consistent double quotes in regex * Add missing newline and __init__.py file * Add missing newline and __init__.py with license * Fix __init__.py with proper license and single newline * Implement pool name normalization for stats reporting Following @potiuk's feedback, changed approach from preventing invalid pool names to normalizing them for stats reporting. Changes: - Replaced validate_pool_name() with normalize_pool_name_for_stats() - Pool names with invalid characters are normalized (replaced with _) when emitting metrics - Logs warning when normalization occurs, suggesting pool rename - Removed validation from create_or_update_pool() - Updated scheduler_job_runner.py to use normalized names for stats - Removed validation tests - Updated news fragment This prevents InvalidStatsNameException without breaking existing pools with invalid names. Fixes apache#59935 * Implement pool name normalization for stats reporting Following @potiuk's feedback, changed approach from preventing invalid pool names to normalizing them for stats reporting. Changes: - Added normalize_pool_name_for_stats() function in pool.py - Pool names with invalid characters are normalized (replaced with _) when emitting metrics in scheduler_job_runner.py - Logs warning when normalization occurs - Removed validation tests - Removed accidental fix_quote.py file - Updated news fragment This prevents InvalidStatsNameException without breaking existing pools. Fixes apache#59935 * Add missing 're' module import * Remove duplicate import * Apply CI formatting fixes - import order and blank lines * Fix formatting - add proper blank lines per ruff format requirements

These were added in apache#59938 and do nothing at all.

kaxil · 2026-01-14T19:22:00Z

+# specific language governing permissions and limitations
+# under the License.
+
+from __future__ import annotations


@kalluripradeep Where are the tests :D ?

Hi @kaxil!

The tests were included in my original PR commits (1edd0d3, 6ff4d64, 0025d3a) but looks like they were removed in #60548 as part of a cleanup.

I see @jedcunningham has opened #60553 to add them back with optimizations. Happy to help review or add more test coverage if needed!

Let me know if you'd like me to contribute additional tests. 👍

These were added in #59938 and do nothing at all.

These were added in apache#59938 and do nothing at all.

@potiuk

…pache#59938) * Add Validation for pool names to prevent InvalidStatsNameException Pool names can now only contain ASCII alphabets, numbers , underscores,dots,and dashes to ensure compatibility with stats naming requirments. fixes apache#59935 * Add unit Tests and news fragment for pool name validation * Fix test file with proper database session handling * Fix test assertions to use pytest.raises match parameter * Add newline at the end of pool.py file * Fix trailing whitespace and quote style in pool.py * Fix syntax error: use consistent double quotes in regex * Add missing newline and __init__.py file * Add missing newline and __init__.py with license * Fix __init__.py with proper license and single newline * Implement pool name normalization for stats reporting Following @potiuk's feedback, changed approach from preventing invalid pool names to normalizing them for stats reporting. Changes: - Replaced validate_pool_name() with normalize_pool_name_for_stats() - Pool names with invalid characters are normalized (replaced with _) when emitting metrics - Logs warning when normalization occurs, suggesting pool rename - Removed validation from create_or_update_pool() - Updated scheduler_job_runner.py to use normalized names for stats - Removed validation tests - Updated news fragment This prevents InvalidStatsNameException without breaking existing pools with invalid names. Fixes apache#59935 * Implement pool name normalization for stats reporting Following @potiuk's feedback, changed approach from preventing invalid pool names to normalizing them for stats reporting. Changes: - Added normalize_pool_name_for_stats() function in pool.py - Pool names with invalid characters are normalized (replaced with _) when emitting metrics in scheduler_job_runner.py - Logs warning when normalization occurs - Removed validation tests - Removed accidental fix_quote.py file - Updated news fragment This prevents InvalidStatsNameException without breaking existing pools. Fixes apache#59935 * Add missing 're' module import * Remove duplicate import * Apply CI formatting fixes - import order and blank lines * Fix formatting - add proper blank lines per ruff format requirements

These were added in apache#59938 and do nothing at all.

@potiuk

…pache#59938) * Add Validation for pool names to prevent InvalidStatsNameException Pool names can now only contain ASCII alphabets, numbers , underscores,dots,and dashes to ensure compatibility with stats naming requirments. fixes apache#59935 * Add unit Tests and news fragment for pool name validation * Fix test file with proper database session handling * Fix test assertions to use pytest.raises match parameter * Add newline at the end of pool.py file * Fix trailing whitespace and quote style in pool.py * Fix syntax error: use consistent double quotes in regex * Add missing newline and __init__.py file * Add missing newline and __init__.py with license * Fix __init__.py with proper license and single newline * Implement pool name normalization for stats reporting Following @potiuk's feedback, changed approach from preventing invalid pool names to normalizing them for stats reporting. Changes: - Replaced validate_pool_name() with normalize_pool_name_for_stats() - Pool names with invalid characters are normalized (replaced with _) when emitting metrics - Logs warning when normalization occurs, suggesting pool rename - Removed validation from create_or_update_pool() - Updated scheduler_job_runner.py to use normalized names for stats - Removed validation tests - Updated news fragment This prevents InvalidStatsNameException without breaking existing pools with invalid names. Fixes apache#59935 * Implement pool name normalization for stats reporting Following @potiuk's feedback, changed approach from preventing invalid pool names to normalizing them for stats reporting. Changes: - Added normalize_pool_name_for_stats() function in pool.py - Pool names with invalid characters are normalized (replaced with _) when emitting metrics in scheduler_job_runner.py - Logs warning when normalization occurs - Removed validation tests - Removed accidental fix_quote.py file - Updated news fragment This prevents InvalidStatsNameException without breaking existing pools. Fixes apache#59935 * Add missing 're' module import * Remove duplicate import * Apply CI formatting fixes - import order and blank lines * Fix formatting - add proper blank lines per ruff format requirements

These were added in apache#59938 and do nothing at all.

@potiuk

…pache#59938) * Add Validation for pool names to prevent InvalidStatsNameException Pool names can now only contain ASCII alphabets, numbers , underscores,dots,and dashes to ensure compatibility with stats naming requirments. fixes apache#59935 * Add unit Tests and news fragment for pool name validation * Fix test file with proper database session handling * Fix test assertions to use pytest.raises match parameter * Add newline at the end of pool.py file * Fix trailing whitespace and quote style in pool.py * Fix syntax error: use consistent double quotes in regex * Add missing newline and __init__.py file * Add missing newline and __init__.py with license * Fix __init__.py with proper license and single newline * Implement pool name normalization for stats reporting Following @potiuk's feedback, changed approach from preventing invalid pool names to normalizing them for stats reporting. Changes: - Replaced validate_pool_name() with normalize_pool_name_for_stats() - Pool names with invalid characters are normalized (replaced with _) when emitting metrics - Logs warning when normalization occurs, suggesting pool rename - Removed validation from create_or_update_pool() - Updated scheduler_job_runner.py to use normalized names for stats - Removed validation tests - Updated news fragment This prevents InvalidStatsNameException without breaking existing pools with invalid names. Fixes apache#59935 * Implement pool name normalization for stats reporting Following @potiuk's feedback, changed approach from preventing invalid pool names to normalizing them for stats reporting. Changes: - Added normalize_pool_name_for_stats() function in pool.py - Pool names with invalid characters are normalized (replaced with _) when emitting metrics in scheduler_job_runner.py - Logs warning when normalization occurs - Removed validation tests - Removed accidental fix_quote.py file - Updated news fragment This prevents InvalidStatsNameException without breaking existing pools. Fixes apache#59935 * Add missing 're' module import * Remove duplicate import * Apply CI formatting fixes - import order and blank lines * Fix formatting - add proper blank lines per ruff format requirements

These were added in apache#59938 and do nothing at all.

Add Validation for pool names to prevent InvalidStatsNameException Po…

52400ac

…ol names can now only contain ASCII alphabets, numbers , underscores,dots,and dashes to ensure compatibility with stats naming requirments. fixes apache#59935

kalluripradeep requested review from XD-DENG and ashb as code owners December 30, 2025 16:19

kalluripradeep added 6 commits December 30, 2025 17:31

Add unit Tests and news fragment for pool name validation

1edd0d3

Fix test file with proper database session handling

6ff4d64

Fix test assertions to use pytest.raises match parameter

0025d3a

Add newline at the end of pool.py file

2764d5c

Fix trailing whitespace and quote style in pool.py

fe36df4

Fix syntax error: use consistent double quotes in regex

72f8b4b

Prajwal7842 reviewed Dec 31, 2025

View reviewed changes

Comment thread airflow-core/src/airflow/models/pool.py Outdated

kalluripradeep added 3 commits December 31, 2025 10:55

Add missing newline and __init__.py file

2638fea

Add missing newline and __init__.py with license

75a4ab8

Fix __init__.py with proper license and single newline

cd86e45

potiuk approved these changes Dec 31, 2025

View reviewed changes

potiuk requested changes Dec 31, 2025

View reviewed changes

potiuk reviewed Dec 31, 2025

View reviewed changes

Comment thread fix_quote.py Outdated

potiuk reviewed Dec 31, 2025

View reviewed changes

Comment thread airflow-core/src/airflow/models/pool.py Outdated

potiuk approved these changes Dec 31, 2025

View reviewed changes

kalluripradeep force-pushed the fix-pool-name-validation branch from ea8a80c to b9bfe4c Compare December 31, 2025 17:26

potiuk approved these changes Dec 31, 2025

View reviewed changes

kalluripradeep added 4 commits January 1, 2026 12:13

Add missing 're' module import

1fe9945

Remove duplicate import

d729929

Apply CI formatting fixes - import order and blank lines

91ab4e9

Fix formatting - add proper blank lines per ruff format requirements

a561035

potiuk mentioned this pull request Jan 1, 2026

Not all pool names align with stats naming restrictions #59935

Closed

2 tasks

potiuk approved these changes Jan 3, 2026

View reviewed changes

potiuk merged commit b7fe726 into apache:main Jan 3, 2026
69 checks passed

jedcunningham added a commit to astronomer/airflow that referenced this pull request Jan 14, 2026

Remove unused "test" files

bd3b5b9

These were added in apache#59938 and do nothing at all.

jedcunningham mentioned this pull request Jan 14, 2026

Remove unused "test" files #60548

Merged

kaxil reviewed Jan 14, 2026

View reviewed changes

jedcunningham mentioned this pull request Jan 14, 2026

Move core newsfragment to right folder; fix release command #60546

Merged

jedcunningham added a commit that referenced this pull request Jan 14, 2026

Remove unused "test" files (#60548)

700f519

These were added in #59938 and do nothing at all.

jedcunningham mentioned this pull request Jan 14, 2026

Optimize pool name normalization for stats and add tests #60553

Merged

1 task

jason810496 pushed a commit to jason810496/airflow that referenced this pull request Jan 22, 2026

Remove unused "test" files (apache#60548)

ba3e45e

These were added in apache#59938 and do nothing at all.

jhgoebbert pushed a commit to jhgoebbert/airflow_Owen-CH-Leung that referenced this pull request Feb 8, 2026

Remove unused "test" files (apache#60548)

639944d

These were added in apache#59938 and do nothing at all.

choo121600 pushed a commit to choo121600/airflow that referenced this pull request Feb 22, 2026

Remove unused "test" files (apache#60548)

6e868af

These were added in apache#59938 and do nothing at all.

Subham-KRLX pushed a commit to Subham-KRLX/airflow that referenced this pull request Mar 4, 2026

Remove unused "test" files (apache#60548)

d4c58a3

These were added in apache#59938 and do nothing at all.

Ankurdeewan pushed a commit to Ankurdeewan/airflow that referenced this pull request Mar 15, 2026

Remove unused "test" files (apache#60548)

8d34521

These were added in apache#59938 and do nothing at all.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add validation for pool names to prevent InvalidStatsNameException#59938

Add validation for pool names to prevent InvalidStatsNameException#59938
potiuk merged 16 commits into
apache:mainfrom
kalluripradeep:fix-pool-name-validation

kalluripradeep commented Dec 30, 2025

Uh oh!

BasPH commented Dec 30, 2025

Uh oh!

kalluripradeep commented Dec 30, 2025

Uh oh!

Uh oh!

potiuk left a comment •

edited

Loading

Uh oh!

potiuk left a comment

Uh oh!

Uh oh!

Uh oh!

potiuk left a comment

Uh oh!

kalluripradeep commented Dec 31, 2025

Uh oh!

potiuk commented Dec 31, 2025

Uh oh!

kalluripradeep commented Jan 1, 2026

Uh oh!

Uh oh!

kaxil Jan 14, 2026

Uh oh!

kalluripradeep Jan 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

kalluripradeep commented Dec 30, 2025

Problem

Solution

Changes

Testing

Uh oh!

BasPH commented Dec 30, 2025

Uh oh!

kalluripradeep commented Dec 30, 2025

Uh oh!

Uh oh!

potiuk left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

potiuk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

potiuk left a comment

Choose a reason for hiding this comment

Uh oh!

kalluripradeep commented Dec 31, 2025

Uh oh!

potiuk commented Dec 31, 2025

Uh oh!

kalluripradeep commented Jan 1, 2026

Uh oh!

Uh oh!

kaxil Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

kalluripradeep Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

potiuk left a comment •

edited

Loading