Rules: Return 404 when DID rules not found #6334

[alexanderrichards]

This PR fixes #6334.

The empty generator on the line

https://github.com/alexanderrichards/rucio/blob/01aa2184041e34584a82669ba846dfedfd8f0f51/lib/rucio/api/rule.py#L163

means that this generator never yields anything but neither does it raise an exception to get caught which would trigger a return value other than 200.

Instead of duplicating the generator and peeking I simply add a counter to see if it ever yields. If not then there were no rules and we can raise RuleNotFound.

[bari12]

Hi @alexanderrichards
While this works, I think it slightly misses the semantics of the original issue.

Thus, if an invalid (= non existent) did is given to the rest endpoint (/dids/{scope_name}/rules) then a DIDNotFound exception should be raised. How you implemented it, even if a valid did is given, an exception is raised that no rules exist, but this is not really exception-worthy, since a did can have no rules.

Now what makes this a bit special is that the core call: list_rules being used is a filter. So calling this normally with filter values, I would not expect it to raise a DIDNotFound, but in the specific circumstances how it is being instrumented in the REST endpoint, I would still expect the rest endpoint to raise that.

So what I would do is at a existence check for the scope:name in the REST call. Thus a get_did call before.

Hope that makes sense :-)

Martin

[alexanderrichards]

Hi @bari12, I understand. I can therefore put this check not in the core layer but either in the api layer (in api/rule.py::list_replication_rules) or REST layer (core/dids.py::Rules::get). Where would you say it makes most sense? Would you expect that the api layer raise DataIdentifierNotFound exception or only from REST?

[bari12]

Yes, that's the two options I see as well.

My feeling is that this is a bit more accurate to be adressed in the REST layer directly. Thus in /dids/{scope_name}/rules you add the existence check of the did.

It's not entirely wrong to do it in api/rule.py::list_replication_rules either, but you would have to do something like check if only scope and name are in the filter, and if this is the case, then also check for did existence. But this feels a tiny bit assumptious to me, since you do not entirely know at that point that only a specific DID (which should exist) is being checked. At the REST layer you do know that, so I would put it there.

[alexanderrichards]

looks like the changes are causing a test to fail because the mocked scope/name combination are clearly not found in the new get_did api call. How would you like to proceed? I can mock this call if you like.

[gw0] linux -- Python 3.9.18 /opt/venv/bin/python
tests/test_rule.py:1379: in test_list_rules_by_did
    ret = rucio_client.list_did_rules(scope=mock_scope.external, name=dataset['name'])
lib/rucio/client/didclient.py:544: in list_did_rules
    raise exc_cls(exc_msg)
E   rucio.common.exception.DataIdentifierNotFound: Data identifier not found.

[bari12]

I think the problem here is that you need to add the VO into the get_did call as well. Similar to

rucio/lib/rucio/web/rest/flaskapi/v1/dids.py

Line 550 in 16bb6f9

did = get_did(scope=scope, name=name, dynamic_depth=dynamic_depth, vo=request.environ.get('vo'))