search

civisanalytics / civis-python

Civis API Python Client
BSD 3-Clause "New" or "Revised" License
34 stars 26 forks source link

Retry on Sharing Endpoint 404s? #271

Closed waltaskew closed 5 years ago

waltaskew commented 5 years ago

The sharing endpoints (put_shares_uers, put_shares_groups) intermittently return 404s. Sometimes they represent a permanent failure, but sometimes they represent an intermittent failure which may be retried. We should consider retrying on 404s in case they are intermittent, but it's hard to know what the best course here would be.

keithing commented 5 years ago

Hmm, wouldn't it be a little unconventional to retry on a 404? I can see two possibilities for a fix, both of which (initially) look dubious to me:

1) Adding a custom retry rule for this specific endpoint 2) Adding a broad rule to retry on 404s

Would this be better fixed at Civis Platform level rather than in this client library? If a failure is intermittent, why would a 404 be returned?

waltaskew commented 5 years ago

Yeah, it's not a great situation. I was just thinking about retrying on 404s on sharing endpoints. The best solution would definitely be if the platform didn't return intermittent 404s on sharing!

mheilman commented 5 years ago

Legitimate 404s happen when one doesn't have permissions on a thing they're trying to share, right? If so, we'd be retrying those requests, too.

waltaskew commented 5 years ago

Legitimate 404s happen when one doesn't have permissions on a thing they're trying to share, right? If so, we'd be retrying those requests, too.

Right, that's the issue.

keithing commented 5 years ago

I'm going to flag this to the Platform engineers, I'd prefer to fix this in the Platform code rather than solidify a workaround in this library.

In the meantime, you can work around with a simple retry in your client code, right?

stephen-hoover commented 5 years ago

Can we close this issue? It doesn't sound like this is a change we want to make.