-
-
Notifications
You must be signed in to change notification settings - Fork 638
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
Don't filter the env passed to OptionsBootstrapper. #20956
Conversation
The native parser needs access to all env vars for interpolation, so we can't filter down to just PANTS_*. Once we're rid of the legacy parser and can simplify options bootstrapping, we can revisit exactly how to do this, but for now we're OK with OptionsBootstrapper holding more data than it strictly needs.
Note that the underlying issue is that native parser uses the env from OptionsBootstrapper for interpolation, whereas the legacy parser parses Config before OptionsBootstrapper is even created. |
Fixes #20952 |
Hint: if you edit that comment into the PR description, GH will auto-close the issue when this gets 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.
Makes sense
On second thoughts, instead of skipping the test I'll make it test the new behavior, so we know if that gets reverted (and to force us to fix the test if/when we do revert it). |
The native parser needs access to all env vars for interpolation,
so we can't filter down to just PANTS_*.
Once we're rid of the legacy parser and can simplify options bootstrapping,
we can revisit exactly how to do this, but for now we're OK with
OptionsBootstrapper holding more data than it strictly needs.