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
Owner

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.

import pynetbox
# stuff
prefixes = client.ipam.prefixes.filter(prefix='10.0.0.0/26', vrf='rd', tag='nonexist')
# throws
# pynetbox.core.query.RequestError: The request failed with code 400 Bad Request: {'tag': ['Select a valid choice. nonexist is not one of the available choices.']}

Database changes

No response

External dependencies

No response

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. ```python import pynetbox # stuff prefixes = client.ipam.prefixes.filter(prefix='10.0.0.0/26', vrf='rd', tag='nonexist') # throws # pynetbox.core.query.RequestError: The request failed with code 400 Bad Request: {'tag': ['Select a valid choice. nonexist is not one of the available choices.']} ``` ### Database changes _No response_ ### External dependencies _No response_
adam added the type: featurestatus: under review labels 2025-12-29 20:21:45 +01:00
adam closed this issue 2025-12-29 20:21:45 +01:00
Author
Owner

@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.

do such tags exist, do such vrf exist, and only then, does the entity exist.

You need to know this either way in order to form the request to begin with, right?

@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. > do such tags exist, do such vrf exist, and only then, does the entity exist. You need to know this either way in order to form the request to begin with, right?
Author
Owner

@Boris-Barboris commented on GitHub (Jan 5, 2023):

You need to know this either way in order to form the request to begin with, right?

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.

@Boris-Barboris commented on GitHub (Jan 5, 2023): > You need to know this either way in order to form the request to begin with, right? 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.
Author
Owner

@jeremystretch commented on GitHub (Jan 5, 2023):

We don't really care about tags, they are only used as query optimizers

My point is that you can't specify a tag without first knowing that it exists.

Why does the user need to write such complex response parsing code to get this answer?

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.

btw, why 400 and not 404?

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.

@jeremystretch commented on GitHub (Jan 5, 2023): > We don't really care about tags, they are only used as query optimizers My point is that you can't specify a tag without first knowing that it exists. > Why does the user need to write such complex response parsing code to get this answer? 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. > btw, why 400 and not 404? 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.
Author
Owner

@Boris-Barboris commented on GitHub (Jan 5, 2023):

My point is that you can't specify a tag without first knowing that it exists.

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..."

You seem to be making a lot of assumptions about how a particular client is implemented.

That is what I had done in my client before switching to redundant does-tag/vrf-exists requests.

@Boris-Barboris commented on GitHub (Jan 5, 2023): > My point is that you can't specify a tag without first knowing that it exists. 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..." > You seem to be making a lot of assumptions about how a particular client is implemented. That is what I had done in my client before switching to redundant does-tag/vrf-exists requests.
Author
Owner

@candlerb commented on GitHub (Jan 20, 2023):

I don't see any justification to change the existing implementation but I'll leave this open for others to comment.

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:

  1. Query if the VRF exists (you can keep a local cache of this if you like)
  2. If it does, then query for VRF and prefix

OR more simply:

  1. Query for the VRF and prefix, and treat a 400 error as meaning that it does not exist
@candlerb commented on GitHub (Jan 20, 2023): > I don't see any justification to change the existing implementation but I'll leave this open for others to comment. 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: 1. Query if the VRF exists (you can keep a local cache of this if you like) 2. If it does, then query for VRF and prefix OR more simply: 1. Query for the VRF and prefix, and treat a 400 error as meaning that it does not exist
Author
Owner

@Boris-Barboris commented on GitHub (Jan 20, 2023):

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.

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.

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.

Forming a prefix response body would require all it's tags anyway, so not much speed is gained.

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.

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).

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:

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.

Query if the VRF exists (you can keep a local cache of this if you like)
If it does, then query for VRF and prefix

That's what we had to do. Unnecessary request.

Query for the VRF and prefix, and treat a 400 error as meaning that it does not exist

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.

@Boris-Barboris commented on GitHub (Jan 20, 2023): > 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. 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. > 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. Forming a prefix response body would require all it's tags anyway, so not much speed is gained. > 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. 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). > 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: 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. > Query if the VRF exists (you can keep a local cache of this if you like) If it does, then query for VRF and prefix That's what we had to do. Unnecessary request. > Query for the VRF and prefix, and treat a 400 error as meaning that it does not exist 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.
Author
Owner

@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.

@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](https://github.com/netbox-community/netbox/blob/develop/CONTRIBUTING.md).
Author
Owner

@jeremystretch commented on GitHub (May 2, 2023):

Closing this out as the proposed change has not found support in the community.

@jeremystretch commented on GitHub (May 2, 2023): Closing this out as the proposed change has not found support in the community.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/netbox#7322