Skip to content
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

fix: ignore unknown env vars #141

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

matthewelwell
Copy link
Contributor

@matthewelwell matthewelwell commented Jan 22, 2025

Related to this issue.

The purpose is to prevent the application from hard crashing when unrecognised environment variables are provided.

@matthewelwell matthewelwell changed the title fix: try ignoring extra in AppConfig fix: ignore unknown env vars Jan 22, 2025
Copy link
Member

@rolodato rolodato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error is not about unknown environment variables, it only happens if you try to use defined aliases

validation_alias=AliasChoices(
"api_poll_frequency_seconds",
"api_poll_frequency",

@@ -136,6 +136,9 @@ class AppSettings(BaseModel):


class AppConfig(AppSettings, BaseSettings):
class Meta:
extra = "ignore"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... yes, but it's overridden specifically in the BaseSettings class as per this.

That being said, I expected (based on this) that something like docker run --rm -e FOO_BAR=foo flagsmith/edge-proxy:2.17.0 would also generate the error, but it didn't.

Probably more investigation is needed, although the solution I added here does prevent the error reported in the issue when using API_POLL_FREQUENCY, but perhaps it also has some other knock on effects that need investigating before (if) we merge this.

@matthewelwell matthewelwell force-pushed the fix/ignore-unkown-env-vars branch from 376f357 to 92a2142 Compare January 22, 2025 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants