Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Check pyproject requirements before running unit tests #169

Merged
merged 9 commits into from
Feb 14, 2025

Conversation

janbuchar
Copy link
Contributor

See https://github.com/apify/crawlee-python/actions/runs/13078683861/job/36501197341 for a practical application

Since it doesn't seem to be possible to reuse code in reusable workflows, I just added this to unit tests...

@janbuchar janbuchar requested review from vdusek and Pijukatel January 31, 2025 20:11
@github-actions github-actions bot added this to the 107th sprint - Tooling team milestone Jan 31, 2025
@github-actions github-actions bot added the t-tooling Issues with this label are in the ownership of the tooling team. label Jan 31, 2025
@vdusek

This comment was marked as resolved.

@Pijukatel
Copy link

I was hoping this would be possible to solve on Poetry level. Looking into Poetry code that prints The currently activated Python version ... is not supported by the project it seems that it can raise exception if we explicitly specify executable (and not look for another Python version). I didn't dig deeper, but maybe we can start Poetry with explicitly passing Python executable and thus trigger this exception if the executable is not compatible?

https://github.com/python-poetry/poetry/blob/main/src/poetry/utils/env/env_manager.py#L421

@janbuchar

This comment was marked as resolved.

@janbuchar
Copy link
Contributor Author

I was hoping this would be possible to solve on Poetry level. Looking into Poetry code that prints The currently activated Python version ... is not supported by the project it seems that it can raise exception if we explicitly specify executable (and not look for another Python version). I didn't dig deeper, but maybe we can start Poetry with explicitly passing Python executable and thus trigger this exception if the executable is not compatible?

Well, I was hoping that you could turn off the autodetection via config 🤷 I also found that code you referenced, but it didn't feel right to depend on undocumented behavior found by digging in the code.

@vdusek
Copy link
Contributor

vdusek commented Feb 3, 2025

Really? The script should check both possible locations for the python version constraint. Is there anything else to handle?

Oh, you're right, I overlooked that, thanks.

Copy link
Contributor

@vdusek vdusek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Pijukatel
Copy link

Pijukatel commented Feb 3, 2025

I was hoping this would be possible to solve on Poetry level. Looking into Poetry code that prints The currently activated Python version ... is not supported by the project it seems that it can raise exception if we explicitly specify executable (and not look for another Python version). I didn't dig deeper, but maybe we can start Poetry with explicitly passing Python executable and thus trigger this exception if the executable is not compatible?

Well, I was hoping that you could turn off the autodetection via config 🤷 I also found that code you referenced, but it didn't feel right to depend on undocumented behavior found by digging in the code.

I think we can use this documented feature: https://python-poetry.org/docs/managing-environments/

I did quick local test with multiple Python versions available and some constraint in Pytproject.toml and it seems to be doing what we want. For example doing it in two steps:

poetry env use python3.10.12
poetry install

-> This will raise error if the requirements do not match selected version

Current Python version (3.10.12) is not allowed by the project (>3.10.12).
Please change python executable via the "env use" command.

@janbuchar
Copy link
Contributor Author

@Pijukatel Since this is non-intrusive and not a huge priority, I'll merge this now. If you want to propose a different solution, feel free to open another PR.

@janbuchar janbuchar merged commit 3dddb91 into main Feb 14, 2025
2 checks passed
@janbuchar janbuchar deleted the check-pyproject-requirements branch February 14, 2025 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t-tooling Issues with this label are in the ownership of the tooling team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI can use wrong Python version in some cases
3 participants