You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "dhkim1920 (via GitHub)" <gi...@apache.org> on 2026/09/03 14:16:06 UTC

[PR] Add tests for common compat standard utilities [airflow]

dhkim1920 opened a new pull request, #72487:
URL: https://github.com/apache/airflow/pull/72487

   Add coverage for the compatibility imports exposed by `common.compat.standard.utils`.
   
   The tests verify every name in `__all__` resolves through the compatibility import map and that unknown attributes retain the expected `AttributeError` contract. This removes the corresponding entry from `OVERLOOKED_TESTS`.
   
   related: #35442
   
   ---
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes — Codex (GPT-5)
   
   Generated-by: Codex (GPT-5) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
   
   ---
   Drafted-by: Codex (GPT-5); reviewed by @dhkim1920 before posting


-- 
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] Add tests for common compat standard utilities [airflow]

Posted by "fat-catTW (via GitHub)" <gi...@apache.org>.
fat-catTW commented on PR #72487:
URL: https://github.com/apache/airflow/pull/72487#issuecomment-5549282479

   Thanks for the contribution!
   
   I think this test should use an independent expected list instead of parametrizing from `utils.__all__`.
   
   Right now `utils.__all__` is derived from `_IMPORT_MAP`, so the test only checks the exports that the implementation currently advertises. If an expected compat export like `prepare_virtualenv`, `SkipMixin`, or one of the XCom constants were accidentally removed from `_IMPORT_MAP`, it would also disappear from `__all__`, and this test would still pass without noticing.
   
   Could we make the expected public exports explicit in the test, then assert both `set(utils.__all__)` and that each expected name resolves successfully? That would make the test protect the actual compatibility contract before removing this file from `OVERLOOKED_TESTS`.


-- 
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] Add tests for common compat standard utilities [airflow]

Posted by "dhkim1920 (via GitHub)" <gi...@apache.org>.
dhkim1920 commented on PR #72487:
URL: https://github.com/apache/airflow/pull/72487#issuecomment-5551610298

   Thanks for the thoughtful review. You're right that deriving the test cases from `__all__` could miss accidental removals. I've updated the test to define the expected exports explicitly, assert that `__all__` matches them, and verify that each export resolves successfully.
   


-- 
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] Add tests for common compat standard utilities [airflow]

Posted by "fat-catTW (via GitHub)" <gi...@apache.org>.
fat-catTW commented on PR #72487:
URL: https://github.com/apache/airflow/pull/72487#issuecomment-5556464753

   Sorry, I missed this before approving.
   
   I noticed the provider compatibility CI is still failing for multiple Airflow versions:
   
   - `Compat 3.0.6:P3.10`
   - `Compat 3.1.8:P3.10`
   - `Compat 3.2.2:P3.10`
   
   Since this PR changes `common-compat` import/export coverage, could you please check whether the newly expected exports are available across these Airflow compatibility environments, or adjust the compatibility handling accordingly?
   


-- 
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] Add tests for common compat standard utilities [airflow]

Posted by "fat-catTW (via GitHub)" <gi...@apache.org>.
fat-catTW commented on PR #72487:
URL: https://github.com/apache/airflow/pull/72487#issuecomment-5565957776

   Great work!!


-- 
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] Add tests for common compat standard utilities [airflow]

Posted by "fat-catTW (via GitHub)" <gi...@apache.org>.
fat-catTW commented on PR #72487:
URL: https://github.com/apache/airflow/pull/72487#issuecomment-5874138540

   The current failing provider compatibility job appears to be an infrastructure/network failure.
   Try to rerun the CI.


-- 
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] Add tests for common compat standard utilities [airflow]

Posted by "dhkim1920 (via GitHub)" <gi...@apache.org>.
dhkim1920 commented on PR #72487:
URL: https://github.com/apache/airflow/pull/72487#issuecomment-5558112979

   Thanks for flagging this. I updated the branch with the latest `main` and reran CI. All provider compatibility jobs now pass, including Airflow 3.0.6, 3.1.8, and 3.2.2. The provider wheel and sdist tests also pass, so no additional compatibility handling appears to be needed for these exports.
   


-- 
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] Add tests for common compat standard utilities [airflow]

Posted by "fat-catTW (via GitHub)" <gi...@apache.org>.
fat-catTW commented on PR #72487:
URL: https://github.com/apache/airflow/pull/72487#issuecomment-5556354087

   Awesome!!
   Nice work 


-- 
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