mirror of
https://github.com/netbox-community/netbox.git
synced 2026-01-11 21:10:29 +01:00
Don't return error when filtering on non-existing tags/vrfs #7322
Closed
opened 2025-12-29 20:21:45 +01:00 by adam
·
8 comments
No Branch/Tag Specified
main
update-changelog-comments-docs
feature-removal-issue-type
20911-dropdown
20239-plugin-menu-classes-mutable-state
21097-graphql-id-lookups
feature
fix_module_substitution
20923-dcim-templates
20044-elevation-stuck-lightmode
feature-ip-prefix-link
v4.5-beta1-release
20068-import-moduletype-attrs
20766-fix-german-translation-code-literals
20378-del-script
7604-filter-modifiers-v3
circuit-swap
12318-case-insensitive-uniqueness
20637-improve-device-q-filter
20660-script-load
19724-graphql
20614-update-ruff
14884-script
02496-max-page
19720-macaddress-interface-generic-relation
19408-circuit-terminations-export-templates
20203-openapi-check
fix-19669-api-image-download
7604-filter-modifiers
19275-fixes-interface-bulk-edit
fix-17794-get_field_value_return_list
11507-show-aggregate-and-rir-on-api
9583-add_column_specific_search_field_to_tables
v4.5.0
v4.4.10
v4.4.9
v4.5.0-beta1
v4.4.8
v4.4.7
v4.4.6
v4.4.5
v4.4.4
v4.4.3
v4.4.2
v4.4.1
v4.4.0
v4.3.7
v4.4.0-beta1
v4.3.6
v4.3.5
v4.3.4
v4.3.3
v4.3.2
v4.3.1
v4.3.0
v4.2.9
v4.3.0-beta2
v4.2.8
v4.3.0-beta1
v4.2.7
v4.2.6
v4.2.5
v4.2.4
v4.2.3
v4.2.2
v4.2.1
v4.2.0
v4.1.11
v4.1.10
v4.1.9
v4.1.8
v4.2-beta1
v4.1.7
v4.1.6
v4.1.5
v4.1.4
v4.1.3
v4.1.2
v4.1.1
v4.1.0
v4.0.11
v4.0.10
v4.0.9
v4.1-beta1
v4.0.8
v4.0.7
v4.0.6
v4.0.5
v4.0.3
v4.0.2
v4.0.1
v4.0.0
v3.7.8
v3.7.7
v4.0-beta2
v3.7.6
v3.7.5
v4.0-beta1
v3.7.4
v3.7.3
v3.7.2
v3.7.1
v3.7.0
v3.6.9
v3.6.8
v3.6.7
v3.7-beta1
v3.6.6
v3.6.5
v3.6.4
v3.6.3
v3.6.2
v3.6.1
v3.6.0
v3.5.9
v3.6-beta2
v3.5.8
v3.6-beta1
v3.5.7
v3.5.6
v3.5.5
v3.5.4
v3.5.3
v3.5.2
v3.5.1
v3.5.0
v3.4.10
v3.4.9
v3.5-beta2
v3.4.8
v3.5-beta1
v3.4.7
v3.4.6
v3.4.5
v3.4.4
v3.4.3
v3.4.2
v3.4.1
v3.4.0
v3.3.10
v3.3.9
v3.4-beta1
v3.3.8
v3.3.7
v3.3.6
v3.3.5
v3.3.4
v3.3.3
v3.3.2
v3.3.1
v3.3.0
v3.2.9
v3.2.8
v3.3-beta2
v3.2.7
v3.3-beta1
v3.2.6
v3.2.5
v3.2.4
v3.2.3
v3.2.2
v3.2.1
v3.2.0
v3.1.11
v3.1.10
v3.2-beta2
v3.1.9
v3.2-beta1
v3.1.8
v3.1.7
v3.1.6
v3.1.5
v3.1.4
v3.1.3
v3.1.2
v3.1.1
v3.1.0
v3.0.12
v3.0.11
v3.0.10
v3.1-beta1
v3.0.9
v3.0.8
v3.0.7
v3.0.6
v3.0.5
v3.0.4
v3.0.3
v3.0.2
v3.0.1
v3.0.0
v2.11.12
v3.0-beta2
v2.11.11
v2.11.10
v3.0-beta1
v2.11.9
v2.11.8
v2.11.7
v2.11.6
v2.11.5
v2.11.4
v2.11.3
v2.11.2
v2.11.1
v2.11.0
v2.10.10
v2.10.9
v2.11-beta1
v2.10.8
v2.10.7
v2.10.6
v2.10.5
v2.10.4
v2.10.3
v2.10.2
v2.10.1
v2.10.0
v2.9.11
v2.10-beta2
v2.9.10
v2.10-beta1
v2.9.9
v2.9.8
v2.9.7
v2.9.6
v2.9.5
v2.9.4
v2.9.3
v2.9.2
v2.9.1
v2.9.0
v2.9-beta2
v2.8.9
v2.9-beta1
v2.8.8
v2.8.7
v2.8.6
v2.8.5
v2.8.4
v2.8.3
v2.8.2
v2.8.1
v2.8.0
v2.7.12
v2.7.11
v2.7.10
v2.7.9
v2.7.8
v2.7.7
v2.7.6
v2.7.5
v2.7.4
v2.7.3
v2.7.2
v2.7.1
v2.7.0
v2.6.12
v2.6.11
v2.6.10
v2.6.9
v2.7-beta1
Solcon-2020-01-06
v2.6.8
v2.6.7
v2.6.6
v2.6.5
v2.6.4
v2.6.3
v2.6.2
v2.6.1
v2.6.0
v2.5.13
v2.5.12
v2.6-beta1
v2.5.11
v2.5.10
v2.5.9
v2.5.8
v2.5.7
v2.5.6
v2.5.5
v2.5.4
v2.5.3
v2.5.2
v2.5.1
v2.5.0
v2.4.9
v2.5-beta2
v2.4.8
v2.5-beta1
v2.4.7
v2.4.6
v2.4.5
v2.4.4
v2.4.3
v2.4.2
v2.4.1
v2.4.0
v2.3.7
v2.4-beta1
v2.3.6
v2.3.5
v2.3.4
v2.3.3
v2.3.2
v2.3.1
v2.3.0
v2.2.10
v2.3-beta2
v2.2.9
v2.3-beta1
v2.2.8
v2.2.7
v2.2.6
v2.2.5
v2.2.4
v2.2.3
v2.2.2
v2.2.1
v2.2.0
v2.1.6
v2.2-beta2
v2.1.5
v2.2-beta1
v2.1.4
v2.1.3
v2.1.2
v2.1.1
v2.1.0
v2.0.10
v2.1-beta1
v2.0.9
v2.0.8
v2.0.7
v2.0.6
v2.0.5
v2.0.4
v2.0.3
v2.0.2
v2.0.1
v2.0.0
v2.0-beta3
v1.9.6
v1.9.5
v2.0-beta2
v1.9.4-r1
v1.9.3
v2.0-beta1
v1.9.2
v1.9.1
v1.9.0-r1
v1.8.4
v1.8.3
v1.8.2
v1.8.1
v1.8.0
v1.7.3
v1.7.2-r1
v1.7.1
v1.7.0
v1.6.3
v1.6.2-r1
v1.6.1-r1
1.6.1
v1.6.0
v1.5.2
v1.5.1
v1.5.0
v1.4.2
v1.4.1
v1.4.0
v1.3.2
v1.3.1
v1.3.0
v1.2.2
v1.2.1
v1.2.0
v1.1.0
v1.0.7-r1
v1.0.7
v1.0.6
v1.0.5
v1.0.4
v1.0.3-r1
v1.0.3
1.0.0
Labels
Clear labels
beta
breaking change
complexity: high
complexity: low
complexity: medium
needs milestone
netbox
pending closure
plugin candidate
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
Mirrored from GitHub Pull Request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: starred/netbox#7322
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
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?
Originally created by @Boris-Barboris on GitHub (Dec 5, 2022).
NetBox version
v3.3.9
Feature type
Change to existing functionality
Proposed functionality
REST-API filters should return empty sets when asked to filter by non-existing tag or VRF, instead of returning 400.
Use case
API is surprising and hard to integrate with because of the answer 'no such entities exist' now is hidden behind cascade of requests: do such tags exist, do such vrf exist, and only then, does the entity exist.
Database changes
No response
External dependencies
No response
@jeremystretch commented on GitHub (Jan 5, 2023):
This is intentional and (IMO) desirable behavior, as it calls immediate attention to the problem rather than leaving the client unsure of whether receiving an empty set of results is correct.
You need to know this either way in order to form the request to begin with, right?
@Boris-Barboris commented on GitHub (Jan 5, 2023):
Not really, my particular code only cares about the entity itself (prefix). We don't really care about tags, they are only used as query optimizers, and I don't want to be querying all filter entities (to verify their existence) before the entity of interest. Attention to the problem is unwarranted as there is no problem.
Here is another argument:
Why does the user need to write such complex response parsing code to get this answer? First he has to custom-handle 400 response code (btw, why 400 and not 404?), then he needs to parse json response body and match it's keys with filter fields he's sent, then he needs to string-match "Select a valid choice..." (do you even guarantee such response format across versions?). This all sounds very janky.
@jeremystretch commented on GitHub (Jan 5, 2023):
My point is that you can't specify a tag without first knowing that it exists.
You seem to be making a lot of assumptions about how a particular client is implemented. For instance, you should never need to "string match" the error message. IMO our approach is quite simple and flexible.
400 is the correct response code for an invalid request. A 404 indicates that a valid request was received for an object that does not exist.
I don't see any justification to change the existing implementation but I'll leave this open for others to comment.
@Boris-Barboris commented on GitHub (Jan 5, 2023):
You can. Example would be a tag that is generated to match some external id in the system above Netbox. Usecase: "find the netbox-side presentation of this prefix. If it exists, according to our tagging logic it would have a tag "tag1". If you have found it, do X, if not, do Y..."
That is what I had done in my client before switching to redundant does-tag/vrf-exists requests.
@candlerb commented on GitHub (Jan 20, 2023):
I agree with @jeremystretch that the current behaviour is correct and important to retain. There are other cases in the REST API where invalid or unexpected query parameters are silently ignored, and these are a pain to troubleshoot. The fewer of these there are, the better.
I think what the OP is doing is called "premature optimisation". OP is saying that the prefix in question, if it exists, would have tag XYZ, if tag XYZ exists. But the OP is also assuming that filtering on the tag and prefix is more efficient than simply filtering on the prefix by itself.
I believe that is incorrect. I say it's almost certainly more efficient to query just for the prefix, since it's fully indexed, and hence will require fewer accesses in the database.
So unless the OP can demonstrate that their version of the query is faster in general (which I doubt they can), they should simply drop the tag match from the query. And that also eliminates the redundant "does-tag exist" query.
As for VRFs, it's a bit different, because (VRF,prefix) is part of the unique identity of a prefix. So you may get many prefixes back. If you think it's likely that query for 192.168.1.0/24 could return (say) a thousand different 192.168.1.0/24 prefixes in different VRFs, then it would be worth optimising. But it's only a question of:
OR more simply:
@Boris-Barboris commented on GitHub (Jan 20, 2023):
Why is non-existing tag invalid or unexpected? It's just non-existing, that's all. It's like expecting SQL to raise error when you've got a non-existing string in SELECT ... WHERE somefield = ?' clause that filters foreign key, I don't understand it. It is perfectly reasonable to throw 400 error when the query filter mentions some non-existing field, but non-existing value... that's what I find surprising and unconventional.
Forming a prefix response body would require all it's tags anyway, so not much speed is gained.
It should not be faster, nor is it my goal. Issue is about optimizing client code size (not matching tags manually, not writing 400 error response body parser) and RPC count (the necessary request count is 1).
Yep, it's a private cloud, ~500 tenants (each lives in it's own vrf) with their private networks that default to 192.168.0.0/24 is my use case. Lot's of similar prefixes.
That's what we had to do. Unnecessary request.
400 is too loose of a code, it can be returned in too many cases. I will not rely on it for this task, nor should anyone.
@github-actions[bot] commented on GitHub (Apr 21, 2023):
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Do not attempt to circumvent this process by "bumping" the issue; doing so will result in its immediate closure and you may be barred from participating in any future discussions. Please see our contributing guide.
@jeremystretch commented on GitHub (May 2, 2023):
Closing this out as the proposed change has not found support in the community.