[pythonic resources][rfc] Adjust EnvVar behavior to allow for direct usage #14488
Conversation
This file contains 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
|
Current dependencies on/for this PR: This comment was auto-generated by Graphite. |
|
I think this is too magical and will end up confusing a different set of users for different reason.. I agree with having the conversion to the underlying environment name being an explicit method for our own internal purposes. I prefer #14490 |
schrockn
requested changes
May 26, 2023
benpankow
added a commit
that referenced
this pull request
Sep 30, 2023
…ow direct usage (#14490) ## Summary Alternative RFC to #14488. Keeps `EnvVar` as a direct subclass of `str` but errors on resolving the value via `__str__` to explicitly prevent users from using `EnvVar` outside of resource/config contexts. Right now we're sort of in the worst of both worlds where a user can use an `EnvVar` in inapproporiate places and they get an odd result. 1. Users who try to use an `EnvVar` value as a string will hit an error: ```python my_env_var = EnvVar("MY_ENV_VAR") # errors! print(f"Hello, {my_env_var}!") ``` 2. Users can directly access the `EnvVar` name or value: ```python my_env_var = EnvVar("MY_ENV_VAR") print(f"Env var {my_env_var.env_var_name} has value {my_env_var.get_value()}") ``` ## Test Plan Set of unit tests, existing unit tests.
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.
Summary
Small RFC in response to #14452. Keeps
EnvVaras a direct subclass ofstrbut implements a custom override of__str__to fetch the value from the environment in the case that the user attempts to use theEnvVaranywhere.In the case that it's passed into a resource or config, the resolution will still happen in a lazy manner.
A potentially compelling alternative could be to error on
__str__to explicitly prevent users from usingEnvVaroutside of resource/config contexts. Right now we're sort of in the worst of both worlds where a user can use anEnvVarin inapproporiate places and they get an odd result (see user comment in the above issue). (See #14490 for an example of this)Test Plan
Small unit test, existing unit tests. Will expand suite if this is a direction we want to go.