-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add env var uppercase validation #6815
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?
Changes from all commits
f65ad62
5844415
933b47b
7521664
324697c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Raise a `ValueError` when an `EnvVar` is declared with a name that is not fully uppercase, so misnamed environment variables are caught at definition time. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -405,7 +405,13 @@ def __init__(self, name: str, default: Any, type_: T) -> None: | |
| name: The environment variable name. | ||
| default: The default value. | ||
| type_: The type of the value. | ||
|
|
||
| Raises: | ||
| ValueError: If the name is not fully uppercase. | ||
| """ | ||
| if not name.isupper(): | ||
| msg = f"Environment variable name must be uppercase: {name!r}" | ||
| raise ValueError(msg) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: This validation turns a previously-accepted configuration into a hard runtime failure for any downstream user. EnvVar is a public class and is also constructed by the env_var descriptor from the declaring class's attribute name, so a user who extends EnvironmentVariables (or directly builds EnvVar) with a snake_case or mixed-case variable name β which was fully supported before β will now hit a ValueError the moment that attribute is accessed. Consider whether a hard failure with no fallback is right for a public API. The repo's own guidance calls for a deprecation window when introducing breaking changes to downstream users (per AGENTS.md/CLAUDE.md). A gentler approach would be to warn during a deprecation period (e.g. console.deprecate) before enforcing the error, or to scope the enforcement so it doesn't reject names that are legitimately intended to be read as-is. Prompt for AI agents |
||
| self.name = name | ||
| self.default = default | ||
| self.type_ = type_ | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.