-
Notifications
You must be signed in to change notification settings - Fork 215
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
Correctly set up constant fields in API responses #3035
Conversation
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.
While the change to the documentation is good, something feels off about using a "method" field to generate a constant value. One alternative might be to create a custom field but that seems like over-engineering.
Approving because I don't know a better way.
Added args that are discarded ( |
490095d
to
242d72f
Compare
242d72f
to
d66de3a
Compare
The swagger docs say, regarding "null" (TIL):
So a field cannot only be null, it seems, and within spec. For the static fields, I wrote this custom field that seems to work: class StaticField(serializers.Field):
def __new__(cls, value, **kwargs):
decorated = extend_schema_field({"type": "string", "nullable": True} if value is None else type(value))(cls)
return super().__new__(decorated, value, **kwargs)
def __init__(self, value, **kwargs):
self.value = value
kwargs["source"] = "*"
kwargs["read_only"] = True
super().__init__(**kwargs)
def to_representation(self):
return self.value The |
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!
Thank you for the implementation and sharing the info on |
Fixes
Fixes #3034 by @obulat
Description
This PR replaces
ReadOnlyField
s withSerializerMethodField
s that return constant values for values that never change and are not derived from models:OEmbedSerializer
:version
,type
ProviderSerializer
:logo_url
This makes the documentation a little bit clearer and fixes the warnings in logs.
Currently,
logo_url
type is given asstring
, but it always returnsnull
. The updated version showsstring or null
. I tried setting it to "only null", but spectacular still throws a warning that it couldn't get the type of the value.string or null
is more accurate thanstring
.OEmbed serializer
Currently, the documentation says that these values are "default", making it seem like a different value is also possible.
In this PR, the documentation says "Value: 1.0", making it clear that it's only a single value.
Testing Instructions
If you run the app on
main
(just up
), and open the documentation (e.g. http://0.0.0.0:50280/v1/#tag/audio/operation/audio_stats), you should see the warnings mentioned in the related issue in the Docker logs.These warnings should not be visible in this PR. The documentation for these fields should be updated to match snapshots in the PR description.
Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin