Check original environment copy for deprecations #7685
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
i'll let alex sign off this one |
a5854f1 to
48d2f47
Compare
thrau
approved these changes
Feb 21, 2023
48d2f47 to
de03048
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This seemed to misbehave previously because we sometimes mutate/set environment variables during startup.
Example
Setting
PERISTENCE=1will still log the deprecation warning forDATA_DIReven though it hasn't been explicitly set.Changes
DATA_DIRenv variable whenPERSISTENCE=1Copy the environment directly after we finish loading any profiles. This copy is then used in when determining deprecations instead of using the current environment.Talked with @alexrashed about this and we decided against this for now. Instead removed the line that was causing the specifically observed issue with the environment variableDATA_DIRbeing set manually as a fallback.@thrau & @giograno Do you remember why this was added? What are the potential regressions we'd be creating by removing this?