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

Swagger API has incorrect documentation for /api/circuits/provider-accounts/ and api/circuits/provider-networks #16765

Closed
aaroneg opened this issue Jun 29, 2024 · 7 comments
Labels
status: duplicate This issue has already been raised topic: OpenAPI type: bug A confirmed report of unexpected behavior in the application

Comments

@aaroneg
Copy link
Contributor

aaroneg commented Jun 29, 2024

Deployment Type

Self-hosted

NetBox Version

v4.0.6

Python Version

3.10

Steps to Reproduce

View the swagger API docs located at https://$hostname/api/schema/swagger-ui/#/circuits/circuits_provider_accounts_create and you will see that the schema for ProviderAccountRequest does not mention any need to specify which provider the account is mapped to, but this field is in fact required.

Expected Behavior

The documentation should show all fields, especially required fields.

Observed Behavior

The required field 'provider' is missing from the documentation.

@aaroneg aaroneg added status: needs triage This issue is awaiting triage by a maintainer type: bug A confirmed report of unexpected behavior in the application labels Jun 29, 2024
@aaroneg
Copy link
Contributor Author

aaroneg commented Jun 29, 2024

Looks like it also affects /api/circuits/provider-networks/

@jeffgdotorg
Copy link
Collaborator

Thank you for reporting this problem in NetBox. I was able to reproduce it in a fresh installation of v4.0.6. I'm moving your issue along to needs owner status.

We're generally aware that there are gaps in our OpenAPI coverage, but currently lack the team capacity to prioritize an effort to address these kinds of problems. If you or another developer with the requisite skills and capacity would like to work it through to a PR, please say so and a maintainer will assign the issue to you.

@jeffgdotorg jeffgdotorg removed their assignment Jul 1, 2024
@jeffgdotorg jeffgdotorg added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation topic: OpenAPI severity: low Does not significantly disrupt application functionality, or a workaround is available and removed status: needs triage This issue is awaiting triage by a maintainer labels Jul 1, 2024
@jeffgdotorg jeffgdotorg changed the title Swagger API has incorrect documentation for /api/circuits/provider-accounts/ Swagger API has incorrect documentation for /api/circuits/provider-accounts/ and api/circuits/provider-networks Jul 1, 2024
@aaroneg
Copy link
Contributor Author

aaroneg commented Jul 1, 2024 via email

@jeffgdotorg
Copy link
Collaborator

I'm not much of a python programmer but if you can whip up a short explainer and it's simple enough i'm willing to give it a shot.

I want your expectations to be realistic – I can't guarantee approval of a PR, but if you're up for a learning exercise, go for it. Addressing these gaps really needs to be done as part of a holistic effort, using a consistent approach that has yet to be fleshed out.

@arthanson
Copy link
Collaborator

This looks potentially like a display issue with spectacular, the field is required in the yaml:

    ProviderAccount:
      type: object
      description: Adds support for custom fields and tags.
      properties:
        id:
          type: integer
          readOnly: true
        url:
          type: string
          format: uri
          readOnly: true
        display:
          type: string
          readOnly: true
        provider:
          $ref: '#/components/schemas/Provider'
        name:
          type: string
          default: ''
          maxLength: 100
        account:
          type: string
          title: Account ID
          maxLength: 100
        description:
          type: string
          maxLength: 200
        comments:
          type: string
        tags:
          type: array
          items:
            $ref: '#/components/schemas/NestedTag'
        custom_fields:
          type: object
          additionalProperties: {}
        created:
          type: string
          format: date-time
          readOnly: true
          nullable: true
        last_updated:
          type: string
          format: date-time
          readOnly: true
          nullable: true
      required:
      - account
      - created
      - display
      - id
      - last_updated
      - provider
      - url

@arthanson arthanson self-assigned this Jul 10, 2024
@arthanson arthanson added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Jul 10, 2024
@arthanson
Copy link
Collaborator

What's happening is the nested=True serializer is getting instantiated first in CircuitSerializer, it looks like Spectacular is caching the fields from this call and using that.

@arthanson
Copy link
Collaborator

closing as duplicate of #16670

@jeremystretch jeremystretch added status: duplicate This issue has already been raised and removed status: accepted This issue has been accepted for implementation severity: low Does not significantly disrupt application functionality, or a workaround is available labels Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate This issue has already been raised topic: OpenAPI type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Development

No branches or pull requests

4 participants