-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add APISettings
setting group
#15701
Conversation
for key in self.model_fields.keys(): | ||
if isinstance(child_settings := getattr(self, key), PrefectBaseSettings): | ||
child_env = child_settings.to_environment_variables( | ||
exclude_unset=exclude_unset, | ||
include_secrets=include_secrets, | ||
) | ||
env_variables.update(child_env) | ||
elif (value := env.get(key)) is not None: | ||
env_variables[ | ||
f"{self.model_config.get('env_prefix')}{key.upper()}" | ||
] = str(value) |
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.
The fix for to_environment_variables
was to iterate over the model_fields
because nested model changes weren't included when exclude_unset=True
.
@model_serializer( | ||
mode="wrap", when_used="always" | ||
) # TODO: reconsider `when_used` default for more control | ||
def ser_model( | ||
self, handler: SerializerFunctionWrapHandler, info: SerializationInfo | ||
) -> Any: |
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.
Moved the model serializer to PrefectBaseSettings
to ensure all nested models respect include_secrets
.
CodSpeed Performance ReportMerging #15701 will not alter performanceComparing Summary
|
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.
lgtm! just a couple comments
Reintroduces changes first introduced in #15580 with fixes to ensure nested models are included when calling
.to_environment_variables()
.See comments below for where this PR differs from #15580.