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

Reduce requests needed for delete() #11

Merged
merged 1 commit into from
May 16, 2018
Merged

Reduce requests needed for delete() #11

merged 1 commit into from
May 16, 2018

Conversation

mzur
Copy link
Collaborator

@mzur mzur commented Apr 24, 2018

This change adds a new getObjectInstance method which creates the Swift object but does not call retrieve(). It can be used for delete() where retrieve() is not required. This saves a HEAD request.

Background:
I observed calls to delete() which threw exceptions because the object to delete does not exist (any more). What I expected in this case was a returned false and no exception. The exception originated from getObject() which fails at the retrieve() call when the object does not exist. My first impulse was to move getObject() inside the try catch block but then I noticed that the retrieve() is not required for delete() in the first place.

This removes the HEAD request that was sent on each delete() call.
@mzur
Copy link
Collaborator Author

mzur commented May 7, 2018

Ping @chrisnharvey.

@mzur
Copy link
Collaborator Author

mzur commented May 15, 2018

Hey @chrisnharvey, sorry for nagging again. If you don't have the time to look after this repository at the moment, feel free to add me as collaborator. I'd be happy to help out with reviewing PRs or issues.

@chrisnharvey
Copy link
Owner

Hi @mzur, so sorry for the delay in responding. This PR got completely lost in my inbox.

I will take a look this evening and merge for you.

I am going to be moving this repository to a new namespace, so I've been thinking about how I'm going to do that without causing too many issues. I'll be happy to add you as a collaborator when I've moved the repo.

@chrisnharvey chrisnharvey merged commit 7dbf016 into chrisnharvey:master May 16, 2018
@chrisnharvey
Copy link
Owner

@mzur This works great! Thank you.

@mzur
Copy link
Collaborator Author

mzur commented May 17, 2018

Thanks! Will you publish a new release as well?

@mzur
Copy link
Collaborator Author

mzur commented Jul 20, 2018

Hey there, any progress with moving this repo to a new namespace or publishing the new release? I'd love to push this version to production to get rid of all the error messages.

@mzur
Copy link
Collaborator Author

mzur commented Aug 21, 2018

@chrisnharvey If you no longer have time to take care of this repo or move it to a new namespace, I'd be happy to maintain a fork from now on, too. Just say the word 😉

@mzur
Copy link
Collaborator Author

mzur commented Sep 4, 2018

I now created the fork mzur/flysystem-openstack-swift with the 0.2.2 release. Anybody who is interested in the most recent version of the package, feel free to use the fork instead.

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.

2 participants