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

add flag --use-env to poetry commands #4179

Closed

Conversation

finswimmer
Copy link
Member

@finswimmer finswimmer commented Jun 14, 2021

The PR adds a new global parameter --use-env. It adds the ability to tell poetry which python executable it should use in the same way poetry env use can do it.

The benefits of the parameter are:

  • running commands in several environments without switching the default python executable first
  • if in-project venvs are used, there is no need to add an entry to the envs.toml

I haven't add any tests until now. Recommendations are welcome :)

Pull Request Check List

Resolves: #issue-number-here

  • Added tests for changed code.
  • Updated documentation for changed code.

@sonarcloud
Copy link

sonarcloud bot commented Jun 15, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@finswimmer finswimmer changed the title add flag --use-env to env commands add flag --use-env to poetry commands Jun 15, 2021
@finswimmer finswimmer marked this pull request as ready for review June 15, 2021 04:45
@finswimmer finswimmer requested a review from a team June 15, 2021 04:45
@finswimmer finswimmer added area/cli Related to the command line kind/feature Feature requests/implementations area/ux Features and improvements related to the user experience labels Jun 15, 2021
@felipou
Copy link

felipou commented Nov 20, 2021

I was about to create a PR with a similar idea that I started coding yesterday, but found this PR when looking for more references now.

My idea was to create a new config virtualenvs.default-executable, which could be a path for a python executable, but with two special values: "system" (the default, keeping the same behavior as today, that is, using sys.executable) and shell which would use shutil.which("python") to get the currently active Python version in the shell. I have no idea if that would be portable, I only use poetry on Linux.

I even wrote some simple code for that, but as I was testing and reviewing, I noticed that the version check code was only for the sys.executable python, so this would require modifying the existing code a lot more (which seemed very risk to me, since it was the first time I was looking at this project's code).

But anyway, I think this would be a lot better (for me at least), since I would only need to change the configuration globally once, and poetry would work as needed for all my project when using pyenv.

@abn abn self-requested a review November 24, 2021 12:55
@finswimmer
Copy link
Member Author

@felipou: I have another proof-of-concept here: #4852 Would be happy if you could test it.

@felipou
Copy link

felipou commented Nov 30, 2021

@felipou: I have another proof-of-concept here: #4852 Would be happy if you could test it.

Interesting, very similar idea. Just by looking at the code, it seems to be working. I'll see if I can test it sometime soon.

I liked how you implemented it without modifying the existing code much, and also how you picked the shell python executable by executing a command instead of using shutil.which as I had suggested. Seems to me like your approach would probably be more portable, but it's just a guess.

@finswimmer
Copy link
Member Author

Closing this in favor of #4852.

@finswimmer finswimmer closed this Jan 17, 2022
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/cli Related to the command line area/ux Features and improvements related to the user experience kind/feature Feature requests/implementations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants