behavior of operations when an item does not exist

[shawkins]

Related to #3121 what should the expectations for operations when the current item does not exist? Some of the logic used getManditory, which will throw a 404. Other operations (like patch and replace) will use fromServer which then performs a get that will return null. Should all operations return null or throw a 404?

cc @rohanKanojia

[rohanKanojia]

I think all operations should return null when item doesn't exist #213 . We also have a require() method which throws an exception if not found.

[shawkins]

Current behavior:

Always a 404 - edit, editStatus, accept, some other operations such as scale, and of course require
null - patch, replace - if the get of the current item returns null, if the item is concurrently deleted before the patch/put, then a 404

ServiceOperationImpl patch/replace are different still #3121

Are you looking for all of these to consistently return null instead?

[rohanKanojia]

All the operations you mention are write operations. Is the behavior the same with read operations?

[rohanKanojia]

I would like to go with the option which doesn't break current behavior that much for users. If we're returning null in case of resource not found we can proceed with all methods returning null instead. If you see majority of operations throwing 404, we can go with 404 one. Or do we want to have a separate strategy for read and write operations?

[shawkins]

Is the behavior the same with read operations?

get returns null, require actually throws a ResourceNotFoundException (which is not a KubernetesClientException) rather than a 404. ​

If you see majority of operations throwing 404, we can go with 404 one.

It looks like 404 is the dominant case. Would you be good if patch/replace throw a 404 when not found?

Or do we want to have a separate strategy for read and write operations?

get behavior seems appropriate and is documented that way in the javadoc. require should throw an exception - but the ResourceNotFoundException seem a little odd in that it can mask the underlying exception. It should at least chain to the KubernetesClientException or just let them be thrown -

kubernetes-client/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/BaseOperation.java

Line 211 in 6a135a7

throw new ResourceNotFoundException("Resource not found : " + e.getMessage());

What other read operations are there to consider?

[manusa]

Not sure of what we have now, but logic says that any replace operation to a non-existent resource should throw an exception.

I'm not that sure about PATCH, and since these methods are newer, having a proper alignment with client-go might be the best approach.

[shawkins]

From what I can see the go client will throw the 404 exception. I'll update #3123 to have more consistency across all operations.