-
Notifications
You must be signed in to change notification settings - Fork 14
Add :native_tls option to Pythonx.uv_init/2 #41
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 :native_tls option to Pythonx.uv_init/2 #41
Conversation
lib/pythonx.ex
Outdated
| - `["--native-tls"]` - use the system's native TLS implementation instead | ||
| of vendored rustls. This is useful in corporate environments where the | ||
| system certificate store must be used. | ||
| - `["--no-cache"]` - disable the cache, forcing a fresh download of all | ||
| packages. | ||
| - `["--reinstall"]` - force reinstallation of all packages, even if they | ||
| are already present. | ||
| - `["--quiet"]` - suppress output from the uv command. |
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.
I would prefer to just have an Elixir option :native_tls. This way we have more control over where how and where we pass specific CLI options. Also, we already have :force, so no need to use --no-cache or --reinstall. @josevalim wdyt?
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.
I think I agree, as we cannot guarantee all of the options will work with Pythonx...
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.
Are there other options you would want as part of this PR or just limit it to :native_tls?
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.
Let's do native tls for now. We can add more options if someone has a need for it :)
b690e95 to
8c0b692
Compare
GenericJam
left a comment
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.
Are we happy with these changes now?
Added support and a bit of validation and tests.
Let me know if you want something different.