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

feat!: use pipx to isolate poetry #63

Merged
merged 6 commits into from
Jan 12, 2024
Merged

feat!: use pipx to isolate poetry #63

merged 6 commits into from
Jan 12, 2024

Conversation

NargiT
Copy link
Contributor

@NargiT NargiT commented May 11, 2023

Following poetry recommendation, it better to use pipx instead of pip to avoid mixing application dependencies with poetry's
https://python-poetry.org/docs/#ci-recommendations

BREAKING CHANGE: Switch using an isolated pipx virtualenv

Copy link
Contributor

@lucsorel lucsorel left a comment

Choose a reason for hiding this comment

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

I would also prefer poetry being installed with pipx for isolation sake. After using curl, it is the 2nd recommended way to install poetry (see https://python-poetry.org/docs/#installing-with-pipx).

Installing poetry using pip can cause dependency clashes with the project's dependencies if the project dependencies are not installed in a virtual environment.

@abatilo
Copy link
Owner

abatilo commented Jul 11, 2023

Thank you for the contribution @NargiT. I'm sorry that this got lost in my inbox. I like the spirit of the change but the changes break CI. Could you take a look?

@NargiT
Copy link
Contributor Author

NargiT commented Jul 26, 2023

I will have a look, but expect some lags @abatilo 🌴

lucsorel
lucsorel approved these changes Aug 5, 2023
Copy link
Contributor

@lucsorel lucsorel left a comment

Choose a reason for hiding this comment

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

Looks good to me (this time, I checked the checkboxes of the files to review)

@lucsorel
Copy link
Contributor

hi @abatilo

I reviewed the changes to this pull request but I don't understand why the process is still blocked 🤔 it looks like the CI must run but I don't understand how to trigger it.

@lucsorel
Copy link
Contributor

@NargiT

After a quick look, it seems that the CI does not work because all tested versions of Poetry do not support the -U option that you use to install Poetry.

Either the obsolete versions of poetry should stop being supported (what is your advice, @abatilo?) or the github action must be tweaked to account for different major versions of poetry 🤷

@abatilo
Copy link
Owner

abatilo commented Aug 27, 2023

@lucsorel Your approval doesn't do anything because you don't have write permissions to this repository.

I believe if @NargiT pushes a new commit to the branch then that should trigger the workflows again.

@NargiT
Copy link
Contributor Author

NargiT commented Aug 28, 2023

hello, I will check the -U option issue

@NargiT
Copy link
Contributor Author

NargiT commented Aug 28, 2023

The "-U" is for pip, not poetry @lucsorel.

Could you tell me which combinaison of python, pip, poetry does not work ? Or simply the github action output ?

@lucsorel
Copy link
Contributor

lucsorel commented Aug 28, 2023

The "-U" is for pip, not poetry

Could you tell me which combinaison of python, pip, poetry does not work ? Or simply the github action output ?

You're totally right! Forgive me, it is pipx that sometimes complains about the -U options: see https://github.com/abatilo/actions-poetry/actions/runs/5524771665 and then click on the sub-jobs that failed, like this one https://github.com/abatilo/actions-poetry/actions/runs/5524771665/job/14960079176.

The jobs always fail for the "latest" version of poetry (as defined in this line of the github actions), which is surprising because -U is a pipx option, not a poetry one 🤔

@NargiT
Copy link
Contributor Author

NargiT commented Nov 24, 2023

This should be good now

@abatilo
Copy link
Owner

abatilo commented Nov 24, 2023

@NargiT None of the required CI workflows have been triggered.

@NargiT
Copy link
Contributor Author

NargiT commented Jan 9, 2024

@NargiT None of the required CI workflows have been triggered.

The maintainer should approve it in order to trigger the github action, I cannot do anything about it 😞

@abatilo abatilo changed the title use pipx to isolate poetry feat!: use pipx to isolate poetry Jan 12, 2024
@abatilo abatilo merged commit 7b6d33e into abatilo:master Jan 12, 2024
76 of 77 checks passed
@lucsorel
Copy link
Contributor

thank you @abatilo for approving the PR, thank you @NargiT for proposing it 😃

@abatilo
Copy link
Owner

abatilo commented Jan 12, 2024

Sorry to all that it took so long to get this finally merged in.

@NargiT NargiT deleted the patch-1 branch January 15, 2024 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants