Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Idempotent option for removing a key that doesn't exist #123

Open
sammaphey opened this issue Feb 3, 2021 · 3 comments
Open

Allow Idempotent option for removing a key that doesn't exist #123

sammaphey opened this issue Feb 3, 2021 · 3 comments

Comments

@sammaphey
Copy link

I have a dictionary like the following:

{'collections': ['RECORD'],
 'data': {'something new': 'data'},
 'metadata': {'any_groups': ['g2'],
              'created_by': 'maphey1',
              'created_date': '2021-02-01 21:02:35.698276+00:00',
              'deleted': False,
              'folder_uri': 'f9d8f48e-0814-11eb-9d94-acde48001122',
              'name': 'Example with description',
              'required_groups': ['maphey1:group', '445-test-3'],
              'schema_uri': '66225a8a-5b45-11eb-b7c1-3af9d3a52e49',
              'updated_by': 'superuser',
              'updated_date': '2021-02-03 13:55:06.201548+00:00'},
 'uri': 'cf2d7d90-64d0-11eb-9019-acde48001122'}

and I want to apply this change:

[{'op': <Operations.remove: 'remove'>, 'path': '/data/another'}]

Which results in a JsonPatchException.

In this line of code it is a requirement that the key be preset on the object in order to apply. In my instance I actually want the ability to skip this (since the key already doesn't exist) which is more idempotent. Is this something that could be don in a PR? Is there some other issue that could be caused by doing this?

@stefankoegl
Copy link
Owner

I don't think that this would conform to RFC 6902 section 5. Do you have some suggestion on how you'd reconcile that?

@sammaphey
Copy link
Author

sammaphey commented Feb 4, 2021

I think this could potentially come down to semantics on what we would deem "successful" (this deems that removing a non-existent key is unsuccessful). The users desire in this case is to remove an object that was not there in the first place. So from the perspective of someone who wants the end state to be met (meaning the property doesn't exist on the document) then the request was "successful" (in my mind) since the end state was met.

A good example of this from HTTP is if you want to remove an item from a database. If the removal found no items you do not receive a 404 instead you get a 204 because the end state was met.

However I know you have no control over the spec and the way you have written the code meets the standards that have been set. So my complaint here is null and void.

Perhaps you could raise a distinct exception that differs from a generic JsonPatchException so that I can catch this specific error and ignore it? Alternatively you could return the op that was failed as an attribute on the exception.

@stefankoegl
Copy link
Owner

We could certainly raise a different exception in that case. Do you want to prepare a pull request?

Another solution to your particular case would be to put a "test" operation first. If the test fails the remove will not be attempted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants