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

Add 'brief' and 'exclude' GET parameters to Swagger Documentation #3457

New Issue
Closed
opened 2025-12-29 18:29:18 +01:00 by adam · 6 comments
adam commented 2025-12-29 18:29:18 +01:00
Owner
Copy Link

Originally created by @dstarner on GitHub (Mar 11, 2020).

Note: The original issue blamed django debug toolbar for slow performance when querying 1000 devices. This was a red herring because it was simply wrapping the numerous SQL calls being made.

Environment

  • Python version: 3.7
  • NetBox version: 2.7.10

The brief and exclude documentation is pretty limited, but extremely powerful, because even just including ?exclude=config_context in my /api/dcim/devices call made the difference from almost 60 second completion to < 5 seconds. This is okay if you dig in deep enough to see that those options exist, but a naive caller will be left sitting there as the context objects are populated / merged. This is also why this long request time was only seen when using the API version when listing 1k devices via the UI.

The NetBox ReadTheDocs briefly mention the brief parameter, but exclude is not mentioned, especially in the context of an expensive request, such as one using config_context. The issue is that although I am familiar with looking through these docs, people who are using our NetBox application internally take the Swagger docs as source of truth, and neither of these options are defined / shown. They then come back and complain to me about the system being slow 😅 It would be nice if the Swagger documentation displayed these values on the appropriate routes.

It may also be worth it to see if the config contexts can be accessed in a nicer way, without needing n database calls for n devices, as this would probably speed things up as well.

The following are potential features that could be based off of these parameters. I can spin up issues if either are seen as remotely possible.

Other Potential Feature 1

Have a configuration option to set if information should be brief by default. In that case, getting the full details could be an expand parameter.

# This could be False by default to maintain current workflow, but if set to true
# then the set of possible requests would return brief information by default
# unless a ?expand=[true|1] is given on the request, in which case it would give details
RETURN_BRIEF_BY_DEFAULT = True # or something named better

Other Potential Feature 2

As a potential feature as well, since its so costly, would it be worth it to make getting the config_context buy-in by default, or at least configurable, where the caller has to explicitly ask for it? This could be done through a GET parameter, or maybe even expand the idea elsewhere and use a library such as [django-rest-framework-serializer-extensions](https://github.com/evenicoulddoit/django-rest-framework-serializer-extensions). Again, simply an idea, let me know what you think. It would be neat to specify to include costly fields only if requested. This would also condense the multiple-serializer views down to just one serializer with the dynamic logic.

Originally created by @dstarner on GitHub (Mar 11, 2020). *Note: The original issue blamed django debug toolbar for slow performance when querying 1000 devices. This was a red herring because it was simply wrapping the numerous SQL calls being made.* ### Environment * Python version: 3.7 * NetBox version: 2.7.10 The `brief` and `exclude` documentation is pretty limited, but **extremely** powerful, because even just including `?exclude=config_context` in my `/api/dcim/devices` call made the difference from almost 60 second completion to < 5 seconds. This is okay if you dig in deep enough to see that those options exist, but a naive caller will be left sitting there as the context objects are populated / merged. This is also why this long request time was only seen when using the API version when listing 1k devices via the UI. The [NetBox ReadTheDocs briefly mention the `brief` parameter](https://netbox.readthedocs.io/en/stable/api/overview/#brief-format), but `exclude` is not mentioned, especially in the context of an expensive request, such as one using `config_context`. The issue is that although I am familiar with looking through these docs, people who are using our NetBox application internally take the Swagger docs as source of truth, and neither of these options are defined / shown. They then come back and complain to me about the system being slow 😅 It would be nice if the Swagger documentation displayed these values on the appropriate routes. It may also be worth it to see if the config contexts can be accessed in a nicer way, without needing `n` database calls for `n` devices, as this would probably speed things up as well. --- The following are potential features that could be based off of these parameters. I can spin up issues if either are seen as remotely possible. ### Other Potential Feature 1 Have a configuration option to set if information should be `brief` by default. In that case, getting the full details could be an `expand` parameter. ```python # This could be False by default to maintain current workflow, but if set to true # then the set of possible requests would return brief information by default # unless a ?expand=[true|1] is given on the request, in which case it would give details RETURN_BRIEF_BY_DEFAULT = True # or something named better ``` ### Other Potential Feature 2 As a potential feature as well, since its so costly, would it be worth it to make getting the `config_context` *buy-in* by default, or at least configurable, where the caller has to explicitly ask for it? This could be done through a GET parameter, or maybe even expand the idea elsewhere and use a library such as `[django-rest-framework-serializer-extensions](https://github.com/evenicoulddoit/django-rest-framework-serializer-extensions)`. Again, simply an idea, let me know what you think. It would be neat to specify to include costly fields only if requested. This would also condense the multiple-serializer views down to just one serializer with the dynamic logic.
adam closed this issue 2025-12-29 18:29:18 +01:00
adam commented 2025-12-29 18:29:19 +01:00
Author
Owner
Copy Link

@lampwins commented on GitHub (Mar 11, 2020):

While I can confirm there have been no changes to the middleware, the current version of NetBox is 2.7.10 and includes some performance improvements. I am also curious if your timing metrics are with caching enabled or disabled?

@lampwins commented on GitHub (Mar 11, 2020): While I can confirm there have been no changes to the middleware, the current version of NetBox is 2.7.10 and includes some performance improvements. I am also curious if your timing metrics are with caching enabled or disabled?
adam commented 2025-12-29 18:29:19 +01:00
Author
Owner
Copy Link

@dstarner commented on GitHub (Mar 11, 2020):

So I have caching enabled in the settings, but it does not seem to be working according to the debug toolbar and the SQL queries I see being performed. Not exactly sure why. I am performing the 2.7.x upgrade today since I know @jeremystretch did some neat optimizations and performance upgrades (thanks!) and I'll be able to tell you how that changes things.

I'm going to upgrade and play around with it, so I'll see what I can figure out.

@dstarner commented on GitHub (Mar 11, 2020): So I have caching enabled in the settings, but it does not seem to be working according to the debug toolbar and the SQL queries I see being performed. Not exactly sure why. I am performing the 2.7.x upgrade today since I know @jeremystretch did some neat optimizations and performance upgrades (thanks!) and I'll be able to tell you how that changes things. I'm going to upgrade and play around with it, so I'll see what I can figure out.
adam commented 2025-12-29 18:29:19 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Mar 11, 2020):

I have several problems with this issue:

  1. It was not reported using a recent release.
  2. It does prescribe an exact data set with which to replicate the reported issue.
  3. You've modified the NetBox code by adding a profiler, with an unknown impact to NetBox itself.
  4. django-debug-toolbar is installed and configured per its documentation, so if there is a significant performance impact resulting from the inclusion of its middleware when debug mode is disabled, that would likely be a bug against django-debug-toolbar, not NetBox. However, the code is pretty quick to pass of the request when it is determined the toolbar should not be displayed, so I don't know where the reported delay would come from.

FWIW, I did some quick benchmarks on v2.7.10 pulling down 1000 devices via the REST API with debug mode disabled. All requests took approximately 12.6s both with and without django-debug-toolbar installed.

@jeremystretch commented on GitHub (Mar 11, 2020): I have several problems with this issue: 1. It was not reported using a recent release. 2. It does prescribe an exact data set with which to replicate the reported issue. 3. You've modified the NetBox code by adding a profiler, with an unknown impact to NetBox itself. 4. django-debug-toolbar is installed and configured per its documentation, so if there _is_ a significant performance impact resulting from the inclusion of its middleware when debug mode is disabled, that would likely be a bug against django-debug-toolbar, not NetBox. However, [the code](https://github.com/jazzband/django-debug-toolbar/blob/master/debug_toolbar/middleware.py#L50-L52) is pretty quick to pass of the request when it is determined the toolbar should not be displayed, so I don't know where the reported delay would come from. FWIW, I did some quick benchmarks on v2.7.10 pulling down 1000 devices via the REST API with debug mode disabled. All requests took approximately 12.6s both with and without django-debug-toolbar installed.
adam commented 2025-12-29 18:29:19 +01:00
Author
Owner
Copy Link

@dstarner commented on GitHub (Mar 11, 2020):

Alright, yea, I am completed the first item this morning. The exact data set is hard to give because of privacy / data concerns since its pretty sensitive data concerning our infrastructure. 😅

As for the profiler, it appeared to add a small linear overhead to the request, but it was proportional to what was being observed before the profiler was added. This means that the slowness was affecting the request with or without the profiler.

For the final point, Im not sure either, I just reported the behavior I saw and the crazy speed up I got when I disabled it. Not really sure....

Anyways, I will upgrade, take another look at where our system is at, and then I'll update this ticket as appropriate or close it if its something on our side. 😃

@dstarner commented on GitHub (Mar 11, 2020): Alright, yea, I am completed the first item this morning. The exact data set is hard to give because of privacy / data concerns since its pretty sensitive data concerning our infrastructure. 😅 As for the profiler, it appeared to add a small linear overhead to the request, but it was proportional to what was being observed before the profiler was added. This means that the slowness was affecting the request with or without the profiler. For the final point, Im not sure either, I just reported the behavior I saw and the crazy speed up I got when I disabled it. Not really sure.... Anyways, I will upgrade, take another look at where our system is at, and then I'll update this ticket as appropriate or close it if its something on our side. 😃
adam commented 2025-12-29 18:29:19 +01:00
Author
Owner
Copy Link

@dstarner commented on GitHub (Mar 12, 2020):

Alright @jeremystretch I concede and recognize that I was chasing a red herring 😅 Turns out the person/people calling the API were unaware of the brief and exclude parameters, so I updated the issue to hopefully be appropriate to the root cause of my issue. Please let me know your thoughts.

@dstarner commented on GitHub (Mar 12, 2020): Alright @jeremystretch I concede and recognize that I was chasing a red herring 😅 Turns out the person/people calling the API were unaware of the `brief` and `exclude` parameters, so I updated the issue to hopefully be appropriate to the root cause of my issue. Please let me know your thoughts.
adam commented 2025-12-29 18:29:19 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Mar 12, 2020):

Please do not rewrite entire issues: Close the existing issue and make a new one if needed. I'm closing and locking this as the discussion history no longer makes sense to the reader.

When opening a new issue, keep in mind that one issue equates to one change. If you want to propose additional changes, each one requires a separate issue.

@jeremystretch commented on GitHub (Mar 12, 2020): Please do not rewrite entire issues: Close the existing issue and make a new one if needed. I'm closing and locking this as the discussion history no longer makes sense to the reader. When opening a new issue, keep in mind that one issue equates to one change. If you want to propose additional changes, each one requires a separate issue.
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
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#3457
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?