Re: [PR] Fix side-effect of running version suffix replacements [airflow]
potiuk merged PR #58937: URL: https://github.com/apache/airflow/pull/58937 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Fix side-effect of running version suffix replacements [airflow]
github-actions[bot] commented on PR #58937: URL: https://github.com/apache/airflow/pull/58937#issuecomment-3629371675 ### Backport successfully created: v3-1-test Status Branch Result ✅ v3-1-test https://github.com/apache/airflow/pull/59223";>https://img.shields.io/badge/PR-59223-blue"; alt="PR Link"> -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Fix side-effect of running version suffix replacements [airflow]
amoghrajesh commented on code in PR #58937:
URL: https://github.com/apache/airflow/pull/58937#discussion_r2597235309
##
dev/breeze/tests/test_packages.py:
##
@@ -375,18 +375,40 @@ def _check_dependencies_modified_properly(
("standard", ".post1", ".post1"),
],
)
-def test_apply_version_suffix_to_provider_pyproject_toml(provider_id,
version_suffix, floored_version_suffix):
+def test_apply_version_suffix_to_provider_pyproject_toml(
Review Comment:
Sounds fine by me
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Fix side-effect of running version suffix replacements [airflow]
potiuk commented on code in PR #58937:
URL: https://github.com/apache/airflow/pull/58937#discussion_r2593748782
##
dev/breeze/tests/test_packages.py:
##
@@ -375,18 +375,40 @@ def _check_dependencies_modified_properly(
("standard", ".post1", ".post1"),
],
)
-def test_apply_version_suffix_to_provider_pyproject_toml(provider_id,
version_suffix, floored_version_suffix):
+def test_apply_version_suffix_to_provider_pyproject_toml(
Review Comment:
Also - I really do not think any of the tests should actually modify the
source tree unless it's actually necessary.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Fix side-effect of running version suffix replacements [airflow]
potiuk commented on code in PR #58937:
URL: https://github.com/apache/airflow/pull/58937#discussion_r2593746886
##
dev/breeze/tests/test_packages.py:
##
@@ -375,18 +375,40 @@ def _check_dependencies_modified_properly(
("standard", ".post1", ".post1"),
],
)
-def test_apply_version_suffix_to_provider_pyproject_toml(provider_id,
version_suffix, floored_version_suffix):
+def test_apply_version_suffix_to_provider_pyproject_toml(
Review Comment:
Lock will potentially serialize those 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Fix side-effect of running version suffix replacements [airflow]
potiuk commented on code in PR #58937:
URL: https://github.com/apache/airflow/pull/58937#discussion_r2593744255
##
dev/breeze/tests/test_packages.py:
##
@@ -375,18 +375,40 @@ def _check_dependencies_modified_properly(
("standard", ".post1", ".post1"),
],
)
-def test_apply_version_suffix_to_provider_pyproject_toml(provider_id,
version_suffix, floored_version_suffix):
+def test_apply_version_suffix_to_provider_pyproject_toml(
Review Comment:
Somehow - I like the current solution more in fact. This one allows for
actual parallelism and we are testing **really** what the method is doing.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Fix side-effect of running version suffix replacements [airflow]
potiuk commented on code in PR #58937:
URL: https://github.com/apache/airflow/pull/58937#discussion_r2593744255
##
dev/breeze/tests/test_packages.py:
##
@@ -375,18 +375,40 @@ def _check_dependencies_modified_properly(
("standard", ".post1", ".post1"),
],
)
-def test_apply_version_suffix_to_provider_pyproject_toml(provider_id,
version_suffix, floored_version_suffix):
+def test_apply_version_suffix_to_provider_pyproject_toml(
Review Comment:
Somehow - I like this better in fact. This one allows for actual parallelism
and we are testing **really** what the method is doing.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Fix side-effect of running version suffix replacements [airflow]
amoghrajesh commented on code in PR #58937:
URL: https://github.com/apache/airflow/pull/58937#discussion_r2582207709
##
dev/breeze/tests/test_packages.py:
##
@@ -375,18 +375,40 @@ def _check_dependencies_modified_properly(
("standard", ".post1", ".post1"),
],
)
-def test_apply_version_suffix_to_provider_pyproject_toml(provider_id,
version_suffix, floored_version_suffix):
+def test_apply_version_suffix_to_provider_pyproject_toml(
Review Comment:
Can we not use an approach using the `lock_version_files` fixture?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Fix side-effect of running version suffix replacements [airflow]
potiuk commented on PR #58937:
URL: https://github.com/apache/airflow/pull/58937#issuecomment-3601921377
> I was thinking of adding an autouse fixture that mocks get_airflow_version
to return a fixed version ("2.10.0"). This would prevent file reads during
parallel execution.
But that would not **really** test the method. What we are really testing
here are the context managers that replace version in the `__init__.py` and
`__pyproject__.toml` and what we **really** want to test is if that replacement
works :)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
