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

Don't automatically reauthenticate on 404 errors #20

Merged
merged 1 commit into from
Dec 10, 2014
Merged

Don't automatically reauthenticate on 404 errors #20

merged 1 commit into from
Dec 10, 2014

Conversation

etaoins
Copy link
Contributor

@etaoins etaoins commented Sep 16, 2014

GET, HEAD, and DELETE were capturing any exception from the lower level
operation and automatically reauthenticating before retrying. The intent
is to catch expired authentication and transparently retry the
operation.

This is inefficient for querying files that don't exist on object
storage as it will trigger the following flow:

  1. GET/HEAD/DELETE is attempted on a non-existent object
  2. The original request fails with an Http_NotFound exception
  3. Reauthentication is performed against Object Storage
  4. The original request is attempted again and the Http_NotFound
    exception is allow to propagate to the caller

Handle this case by explicitly catching an Http_NotFound exception and
rethrowing it at step #2 above

GET, HEAD, and DELETE were capturing any exception from the lower level
operation and automatically reauthenticating before retrying. The intent
is to catch expired authentication and transparently retry the
operation.

This is inefficient for querying files that don't exist on object
storage as it will trigger the following flow:

1) GET/HEAD/DELETE is attempted on a non-existent object
2) The original request fails with an Http_NotFound exception
3) Reauthentication is performed against Object Storage
4) The original request is attempted again and the Http_NotFound
   exception is allow to propagate to the caller

Handle this case by explicitly catching an Http_NotFound exception and
rethrowing it at step #2 above
@petrkotek
Copy link
Contributor

👍 (maybe adding tests around this would be nice though)

ping @briancline

@briancline
Copy link
Member

Thanks for the reminder. Yeah, tests would be good for this. I'll go ahead and merge for now. Thank you for the patch!

briancline added a commit that referenced this pull request Dec 10, 2014
Don't automatically reauthenticate on 404 errors
@briancline briancline merged commit b2ffca9 into softlayer:master Dec 10, 2014
@petrkotek
Copy link
Contributor

thanks for taking care of the pull requests! 🎉

@briancline
Copy link
Member

Sure thing!

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

Successfully merging this pull request may close these issues.

None yet

3 participants