-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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: Added option to disable SSL verify #3676
Feat: Added option to disable SSL verify #3676
Conversation
…s changes to poetry-core also...
… option-to-disable-ssl-verify-develop
Hi, When this feat will be released? Thanks in advance |
Being able to disable SSL verification is whats keeping me from using poetry since we have a private repository. What can I do to help get this pull request merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about fixing the typos in get_trusted right away?
Fixed typo from get_truested to get_trusted Co-authored-by: f <[email protected]>
tests/utils/test_helpers.py
Outdated
trusted = "true" | ||
config.merge({"certificates": {"foo": {"trusted": trusted}}}) | ||
|
||
assert get_truested(config, "foo") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there seems to be a typo here as well: get_trusted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! However it seems more must be done as part of this change:
RuntimeError: The Poetry configuration is invalid: E - [source.0] Additional properties are not allowed ('trusted' was unexpected)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is expected, as written in the original post, in order for tests to pass you must first complete the mentioned PR in poetry-core
as it contains the required changes to the poetry configuration schema.
….com/maayanbar13/poetry into option-to-disable-ssl-verify-develop
I've Reverted the changes according to python-poetry/poetry-core#80. Now disabling SSL verify is only through poetry config certificates.foo.verify False |
If I can help get this merged, please let me know! I'm unable to use Poetry at work without it. |
have the same trouble, if it can help to merge, i'm available to help. I need this feature to use private repository at the office. |
@maayanbar13 thanks for working on this. As we are slowly improving how the http interactions are managed, I'd expect this to go via the authenticator now rather than being configured at the repository instance level. We want to make sure that the requests going to this url is also trusted, similar to how auth and certs are applied. The repository instances can expose this property similar to authenticated url property. |
Let me know when this will be released and if there is anything I can help with to move this along. |
Please refer to the above comment -- this is unable to be merged as is, but recent refactors have made possible a more idiomatic and clean way to introduce this functionality. |
…ting passage to the correct location
@maayanbar13 @neersighted I have rolled up the functional change requested here with a few other structural changes to cert handling in #5719. |
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. |
Pull Request Check List
Resolves: #1556
I'm opening this new pull request to replace /pull/2912 since @Celeborn2BeAlive is no longer following the issue.