panoptes-python-client icon indicating copy to clipboard operation
panoptes-python-client copied to clipboard

Unable to subject.save simple metadata addition

Open PmasonFF opened this issue 6 years ago • 11 comments

This simple metadata correction code is not working...

Panoptes.connect(username=os.environ['User_name'], password=os.environ['Password'])
subjects = Subject.where(subject_set_id=67143)
for subject in subjects:
    subject.metadata['Volume'] = '5'
    subject.save()

Traceback (most recent call last): File "C:/py_scripts/Scripts_WoW/subject_update_metadata_from_file_simplified.py", line 10, in subject.save() File "C:\Users\User1\AppData\Local\Programs\Python\Python37-32\lib\site-packages\panoptes_client\subject.py", line 143, in save log_args=False, File "C:\Users\User1\AppData\Local\Programs\Python\Python37-32\lib\site-packages\redo_init_.py", line 185, in retry return action(*args, **kwargs) File "C:\Users\User1\AppData\Local\Programs\Python\Python37-32\lib\site-packages\panoptes_client\panoptes.py", line 815, in save etag=self.etag File "C:\Users\User1\AppData\Local\Programs\Python\Python37-32\lib\site-packages\panoptes_client\panoptes.py", line 362, in put retry=retry, File "C:\Users\User1\AppData\Local\Programs\Python\Python37-32\lib\site-packages\panoptes_client\panoptes.py", line 281, in json_request json_response['errors'] panoptes_client.panoptes.PanoptesAPIException: JsonApiController::PreconditionFailed

This workaround does work: Update This ran without errors but did not add a field "Volume" to the subject metadata. It seems I can no longer update subject metadata on existing subjects???

Panoptes.connect(username=os.environ['User_name'], password=os.environ['Password'])
subjects = Subject.where(subject_set_id=67143)
for subject in subjects:
    print(subject.id)
    subj = Subject.find(subject.id)
    subj.metadata['Volume'] = '5'    
    subj.save()

PmasonFF avatar Jul 04 '19 22:07 PmasonFF

Even the original code now seems to be working!

Close the issue.

PmasonFF avatar Jul 05 '19 00:07 PmasonFF

I'm reopening this as I just ran into the same issue. It looks like sometimes (but not always) the etag returned by .where() isn't valid when actually trying to save the subject.

Calling .reload() before making any modifications is an easy way to work around it.

adammcmaster avatar Nov 06 '20 10:11 adammcmaster

To be clear, this isn't an API bug. The client should silently reload the subject as needed.

adammcmaster avatar Nov 06 '20 10:11 adammcmaster

I'm reopening this as I just ran into the same issue. It looks like sometimes (but not always) the etag returned by .where() isn't valid when actually trying to save the subject.

Calling .reload() before making any modifications is an easy way to work around it.

OOI - the created 201 and any update 200 response should include the updated etag for the resource. I assume calling reload() won't hit the API again but rather just refresh this internal etag state, right?

camallen avatar Nov 06 '20 13:11 camallen

reload() does hit the API again: https://github.com/zooniverse/panoptes-python-client/blob/master/panoptes_client/panoptes.py#L834 It's the same as calling .find(id) again.

I'm not sure why the etag is wrong sometimes when it's usually fine most of the time.

adammcmaster avatar Nov 06 '20 13:11 adammcmaster

i wonder if the resource is being updated in the API and thus changing the etag value? I can't think what would be updating the subject behind the scenes API side.... but something might touch that resource.

It would be a waste to add a reload on each save event as it's a double round trip to the API and most likely not needed 90% of the time so would slow client calls down and possibly double the API load for these actions.

Could the client rescue this specific error and retry with a reload() once say, then raise if still errors? This would make the client more robust and cut down on extra API calls / round trips.

camallen avatar Nov 06 '20 14:11 camallen

Could the client rescue this specific error and retry with a reload() once say, then raise if still errors? This would make the client more robust and cut down on extra API calls / round trips.

I wouldn't want that to happen automatically in the client because it could be dangerous to overwrite whatever changes there could have been, but that's what I'll do to work around the issue here. I'm not really sure what the solution is -- maybe an explicit option on .save() to allow the client to overwrite changes if you know it's safe?

adammcmaster avatar Nov 06 '20 14:11 adammcmaster

For reference here's how I'm working around it:

                    for retry in (True, False):
                        try:
                            ... make changes to subject ...

                            subject.save()
                        except PanoptesAPIException:
                            if retry:
                                # Reload the subject to refresh the etag
                                # This works around what seems to be a bug in the Panoptes client
                                # https://github.com/zooniverse/panoptes-python-client/issues/220
                                subject.reload()
                            else:
                                raise

adammcmaster avatar Nov 06 '20 14:11 adammcmaster

Ideally we find the source of this bug eventually but it looks like your workaround will help folks encountering this issue.

camallen avatar Nov 06 '20 14:11 camallen

One problem with this approach is that the client DOES retry the failing requests with a backoff interval, but they always fail until you call reload(), so it takes the best part of a minute to save each subject this way. I haven't found a good way around that yet unfortunately.

adammcmaster avatar Nov 06 '20 15:11 adammcmaster

In fact with the backoff it would be fewer requests to just call reload first every time.

adammcmaster avatar Nov 06 '20 15:11 adammcmaster