Add validate_dst_bucket param to key copy functions #399

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants

rpstac commented Nov 10, 2011

This pull request includes a change that enables users of boto to avoid extra AWS bucket list requests when copying keys or changing the storage type of keys. It is set by default to preserve the existing behaviour.

@@ -19,7 +19,8 @@
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
# IN THE SOFTWARE.
-def bucket_lister(bucket, prefix='', delimiter='', marker='', headers=None):
+def bucket_lister(bucket, prefix='', delimiter='', marker='',
+ headers=None, max_keys=1000):
@garnaat

garnaat Nov 18, 2011

Owner

I don't understand the need for the max_keys parameter. It doesn't seem to be used by any of your other changes.

@rpstac

rpstac Nov 18, 2011

Fair point, the pull request should have only been the first two changes. Hadn't realised that it had got in. It does let you retrieve more than the default number of keys at a time if your iterating through a very large bucket and want to save some requests.

@garnaat

garnaat Nov 18, 2011

Owner

You can already do that by passing a "max_keys" param to bucket.get_all_keys() method. This actually adds the "max-keys" param to the query string of the request so the results are limited on the server side.

I don't think the above change will cause any problems. Let me review it a bit more thoroughly and if I don't see a problem, I will include it in the merge. Otherwise, I'll just include the other changes related to verifying the bucket.

@rpstac

rpstac Nov 18, 2011

Yeah thanks, I was using the generator directly hence the extra change. Thanks.

Contributor

Idem commented Jul 12, 2012

This is not only a performance issue.
You currently can not copy a key using this method if you don't have the s3:ListBucket action granted on the bucket's root.

+1 for this param

@garnaat garnaat closed this in 6883f2d Jul 12, 2012

Owner

garnaat commented Jul 12, 2012

I have merged the validate_dst_bucket part of this pull request.

Contributor

Idem commented Jul 12, 2012

awesome, thank!

msabramo pushed a commit to msabramo/boto that referenced this pull request Nov 28, 2012

dyzsasd pushed a commit to dailymotion/boto that referenced this pull request May 9, 2017

Merge pull request #399 from kyleknap/setup-fix
Update futures dependency in exta_require
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment