-
-
Notifications
You must be signed in to change notification settings - Fork 34.2k
Make --use-system-ca per-env rather than per-process #60678
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
base: main
Are you sure you want to change the base?
Conversation
|
Review requested:
|
d87558c to
23473b9
Compare
|
Hmm, I think this may need more work than just updating the options - the implication of being per-env is that each worker would then be able to toggle this independently. Say when the main thread does not enable it but a worker does, then the worker will have the system CA certs in their default store but the parent doesn't. Can you add a test for this, and the other way around (parent enables it, worker disables it)? My impression is that the default store initialisation code is not yet ready for this and it's still shared across the process (so if a worker enables it, suddenly the parent get it too, which would be unexpected). |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #60678 +/- ##
==========================================
- Coverage 88.53% 88.53% -0.01%
==========================================
Files 703 703
Lines 208496 208547 +51
Branches 40213 40216 +3
==========================================
+ Hits 184601 184635 +34
- Misses 15915 15929 +14
- Partials 7980 7983 +3
🚀 New features to boost your workflow:
|
f22457c to
780c272
Compare
3928cbe to
38913f9
Compare
38913f9 to
d9fb49d
Compare
Makes the
--use-system-caoption a per-environment option rather than a per-process option so that workers can enable/disable them individually