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

Lazy Keyring intialization for PasswordManager #1892

Merged
merged 1 commit into from Jan 20, 2020

Conversation

@k4nar
Copy link
Contributor

k4nar commented Jan 15, 2020

When one specify a source in the pyproject.toml, a PasswordManager instance is initialized in the factory function, in order to extract the Basic Auth credentials from the config. While this is fine, this had the undesirable effect to instantiate the Keyring, even when it's not needed.

I had two concerns with this:

  • I don't have a keyring on my system, so every Poetry command was printing a warning. I don't really want this warning unless I'm doing stuff related to credentials, like poetry publish
  • Initializing the keyring adds a small delay to Poetry startup. A very rough estimation on my computer tells me it's about 50 to 100ms, it might be more on slower systems.

Another solution would be to make the parsing of parts of the configuration lazy, so that we don't lead the sources if we don't need to. But that's way more intrusive & bug prone.

@sdispater

This comment has been minimized.

Copy link
Member

sdispater commented Jan 20, 2020

@k4nar Thanks for your contribution and for fixing this. In my opinion, this is a bug fix so this should be based on the master branch.

Could you rebase your branch on master?

@k4nar k4nar force-pushed the k4nar:lazy-keyring branch from c7e40e8 to 79b3f51 Jan 20, 2020
@k4nar k4nar changed the base branch from develop to master Jan 20, 2020
@k4nar

This comment has been minimized.

Copy link
Contributor Author

k4nar commented Jan 20, 2020

Rebased 👍 .

Copy link
Member

sdispater left a comment

LGTM 👍

Thanks!

@sdispater sdispater merged commit 380e09b into python-poetry:master Jan 20, 2020
16 checks passed
16 checks passed
Linting
Details
Linux (2.7)
Details
Linux (3.5)
Details
Linux (3.6)
Details
Linux (3.7)
Details
Linux (3.8)
Details
MacOS (2.7)
Details
MacOS (3.5)
Details
MacOS (3.6)
Details
MacOS (3.7)
Details
MacOS (3.8)
Details
Windows (2.7)
Details
Windows (3.5)
Details
Windows (3.6)
Details
Windows (3.7)
Details
Windows (3.8)
Details
MrGreenTea added a commit to MrGreenTea/poetry that referenced this pull request Feb 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.