-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
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.58% 88.52% -0.07%
==========================================
Files 704 703 -1
Lines 207826 208396 +570
Branches 40049 40190 +141
==========================================
+ Hits 184112 184479 +367
- Misses 15757 15935 +178
- Partials 7957 7982 +25
🚀 New features to boost your workflow:
|
23473b9 to
f22457c
Compare
f22457c to
780c272
Compare
Makes the
--use-system-caoption a per-environment option rather than a per-process option so that workers can enable/disable them individually