starred/netbox
1
0
Fork 0
mirror of https://github.com/netbox-community/netbox.git synced 2026-01-11 21:10:29 +01:00
Code Issues 273 Packages Projects Releases 10 Wiki Activity

Fields conveying annotated values are missing from REST API serialization of newly-created objects #9161

New Issue
Closed
opened 2025-12-29 20:46:23 +01:00 by adam · 4 comments
adam commented 2025-12-29 20:46:23 +01:00
Owner
Copy Link

Originally created by @kendallzhu on GitHub (Jan 26, 2024).

Originally assigned to: @arthanson on GitHub.

Deployment Type

Self-hosted

NetBox Version

v3.7.1

Python Version

3.11

Steps to Reproduce

  1. Run the server
  2. Generate the openapi spec by hitting api/schema endpoint
    (observe "tenant_count" field listed as required for the TenantGroup spec)
  3. Run a creation query for TenantGroup (observe the response does not contain tenant_count field)

This can also be reproduced for many of the other readonly fields in the schema.

Expected Behavior

Expected the generated spec and behavior to match, in one of two ways:

  • Creation query response should contain tenant_count field
  • "tenant_count" should not appear in the required fields for TenantGroup in the openapi spec
    (Similarly for many other readonly fields in the schema)

I suspect it is easier to change the spec than the behavior. It can be done by adding the drf-spectacular setting COMPONENT_NO_READ_ONLY_REQUIRED in the default settings.py, and marking all readonly fields that are required explicitly as required=True.

Relevant line of code specifying serializer properties: https://github.com/netbox-community/netbox/blob/develop/netbox/tenancy/api/serializers.py#L22

Note that setting required='False' in the line^ above does NOT change the generated spec because drf-spectacular defaults to applying a heuristic that readonly fields are required, unless COMPONENT_NO_READ_ONLY_REQUIRED is set. That is why we think the solution is to flip this default and label readonly fields as required except for the ones that will sometimes not be returned.
(More details here: https://github.com/tfranzel/drf-spectacular/issues/1155).

Furthermore I think the fields that are sometimes not returned are the ones added to responses via annotations, such as the one here: b9cac97b73/netbox/tenancy/api/views.py (L41)

For full context, the use case here is deriving generated code from the generated openapi spec, which would be easier if the spec matched with server behavior. Currently it requires manually editing the spec to adjust the aforementioned fields after observing errors.

Observed Behavior

See "Steps to Reproduce"

Originally created by @kendallzhu on GitHub (Jan 26, 2024). Originally assigned to: @arthanson on GitHub. ### Deployment Type Self-hosted ### NetBox Version v3.7.1 ### Python Version 3.11 ### Steps to Reproduce 1. Run the server 2. Generate the openapi spec by hitting api/schema endpoint (observe "tenant_count" field listed as required for the TenantGroup spec) 3. Run a creation query for TenantGroup (observe the response does not contain tenant_count field) This can also be reproduced for many of the other readonly fields in the schema. ### Expected Behavior Expected the generated spec and behavior to match, in one of two ways: - Creation query response should contain tenant_count field - "tenant_count" should not appear in the required fields for TenantGroup in the openapi spec (Similarly for many other readonly fields in the schema) I suspect it is easier to change the spec than the behavior. It can be done by adding the drf-spectacular setting COMPONENT_NO_READ_ONLY_REQUIRED in the default settings.py, and marking all readonly fields that **are** required explicitly as required=True. Relevant line of code specifying serializer properties: https://github.com/netbox-community/netbox/blob/develop/netbox/tenancy/api/serializers.py#L22 Note that setting required='False' in the line^ above does NOT change the generated spec because drf-spectacular defaults to applying a heuristic that readonly fields are required, unless COMPONENT_NO_READ_ONLY_REQUIRED is set. That is why we think the solution is to flip this default and label readonly fields as required _except_ for the ones that will sometimes not be returned. (More details here: https://github.com/tfranzel/drf-spectacular/issues/1155). Furthermore I think the fields that are sometimes not returned are the ones added to responses via annotations, such as the one here: https://github.com/netbox-community/netbox/blob/b9cac97b73c1c08213db6272de11c03711491486/netbox/tenancy/api/views.py#L41 For full context, the use case here is deriving generated code from the generated openapi spec, which would be easier if the spec matched with server behavior. Currently it requires manually editing the spec to adjust the aforementioned fields after observing errors. ### Observed Behavior See "Steps to Reproduce"
adam added the type: bugstatus: acceptedseverity: medium labels 2025-12-29 20:46:23 +01:00
adam closed this issue 2025-12-29 20:46:23 +01:00
adam commented 2025-12-29 20:46:24 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Feb 21, 2024):

  1. Generate the openapi spec by hitting api/schema endpoint
    (observe "tenant_count" field listed as required for the TenantGroup spec)

Where are you seeing tenant_count listed as a required field?

screenshot

AFAICT it's only marked as a required field for the response (which is correct; it will always be included):

screenshot

@jeremystretch commented on GitHub (Feb 21, 2024): > 2. Generate the openapi spec by hitting api/schema endpoint (observe "tenant_count" field listed as required for the TenantGroup spec) Where are you seeing `tenant_count` listed as a required field? ![screenshot](https://github.com/netbox-community/netbox/assets/13487278/c34e8f1b-2d53-4f62-9f0e-2086e5981f07) AFAICT it's only marked as a required field for the response (which is correct; it will always be included): ![screenshot](https://github.com/netbox-community/netbox/assets/13487278/f49dc1e5-58f7-4544-aaeb-e99ed06c3ab9)
adam commented 2025-12-29 20:46:24 +01:00
Author
Owner
Copy Link

@kendallzhu commented on GitHub (Feb 22, 2024):

The issue is that tenant_count does not appear in the response for a creation request, even though it is listed as required.

@kendallzhu commented on GitHub (Feb 22, 2024): The issue is that tenant_count does not appear in the response for a creation request, even though it is listed as required.
adam commented 2025-12-29 20:46:24 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Feb 22, 2024):

Ok, so this has nothing to do with the spec; the field should always exist. The issue is that the field is missing from the object's serialized representation immediately upon being created.

I suppose the root issue is in the create() method of DRF's CreateModelMixin, which uses only the POSTed data when rendering the serializer, rather than the saved object's new database record:

a45432b54d/rest_framework/mixins.py (L18-L23)

The update logic is similarly affected.

We could potentially override this logic to read the newly-created object from the database rather than relying on the input data, which seems like a good idea, however we'll need to consider the ramifications of doing so.

@jeremystretch commented on GitHub (Feb 22, 2024): Ok, so this has nothing to do with the spec; the field _should_ always exist. The issue is that the field is missing from the object's serialized representation immediately upon being created. I suppose the root issue is in the `create()` method of [DRF's `CreateModelMixin`](https://github.com/encode/django-rest-framework/blob/a45432b54de88b28bd2e6db31acfe3dbcfc748dd/rest_framework/mixins.py#L18), which uses only the POSTed data when rendering the serializer, rather than the saved object's new database record: https://github.com/encode/django-rest-framework/blob/a45432b54de88b28bd2e6db31acfe3dbcfc748dd/rest_framework/mixins.py#L18-L23 The update logic is similarly affected. We could potentially override this logic to read the newly-created object from the database rather than relying on the input data, which seems like a good idea, however we'll need to consider the ramifications of doing so.
adam commented 2025-12-29 20:46:25 +01:00
Author
Owner
Copy Link

@jqueuniet commented on GitHub (Mar 8, 2024):

Some users of the netbox-go library seem to have run into a similar issue with the following endpoints:

  • dcim/manufacturers
  • dcim/device-roles
  • dcim/device-types
  • dcim/sites

The netbox-go library is generated from the OpenAPI spec, and enforces the required status of the various missing *_count fields. So this leads to a hard deserialization error right now.

For reference:

https://github.com/netbox-community/go-netbox/issues/165
https://github.com/netbox-community/go-netbox/discussions/166#discussioncomment-8693281

@jqueuniet commented on GitHub (Mar 8, 2024): Some users of the netbox-go library seem to have run into a similar issue with the following endpoints: * dcim/manufacturers * dcim/device-roles * dcim/device-types * dcim/sites The netbox-go library is generated from the OpenAPI spec, and enforces the required status of the various missing `*_count` fields. So this leads to a hard deserialization error right now. For reference: https://github.com/netbox-community/go-netbox/issues/165 https://github.com/netbox-community/go-netbox/discussions/166#discussioncomment-8693281
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
beta
breaking change
complexity: high
complexity: low
complexity: medium
needs milestone
netbox
pending closure
plugin candidate
pull-request
Mirrored from GitHub Pull Request
 severity: high
severity: low
severity: medium
status: accepted
status: backlog
status: blocked
status: duplicate
status: needs owner
status: needs triage
status: revisions needed
status: under review
topic: GraphQL
topic: Internationalization
topic: OpenAPI
topic: UI/UX
topic: cabling
topic: event rules
topic: htmx navigation
topic: industrialization
topic: migrations
topic: plugins
topic: scripts
topic: templating
topic: testing
type: bug
type: deprecation
type: documentation
type: feature
type: housekeeping
type: translation
No Label severity: medium status: accepted type: bug
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/netbox#9161
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?