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

max_keys option of objects.find_all() is ignored #49

Closed
TelegramSam opened this issue Jul 11, 2011 · 3 comments · Fixed by #50
Closed

max_keys option of objects.find_all() is ignored #49

TelegramSam opened this issue Jul 11, 2011 · 3 comments · Fixed by #50

Comments

@TelegramSam
Copy link
Contributor

The fix for Issue #25 has introduced a bug that ignores max_keys.

Currently, find_all() will return all objects in the bucket, obeying prefix and marker.

This happens because the truncated flag is set as soon as the limit is reached, either by the default limit of 1000 or what you send in the max_keys option.

Further, performance is hindered, because it will only request up to max_keys at a time, so a max_keys of 100 will cause 10 request to happen instead of 1 when listing a bucket that returns 1000 keys.

The fix for #25 needs to be adjusted to allow for max_keys set as an option, and stop looping when the limit has been reached.

@qoobaa
Copy link
Owner

qoobaa commented Jul 12, 2011

Hey!

If you create a patch for that I'll appreciate it. I haven't used the library for a while, I don't follow changes of the S3 API recently. I'm quite busy currently, I'll try to find some time to take a look at the library.

Thanks for patches and bug reports.

@TelegramSam
Copy link
Contributor Author

I submitted a pull request on github that contains the written fix. Is
that what you need, or a separate patch file?

-Sam

On Tue Jul 12 03:04:14 2011, qoobaa wrote:

Hey!

If you create a patch for that I'll appreciate it. I haven't used the library for a while, I don't follow changes of the S3 API recently. I'm quite busy currently, I'll try to find some time to take a look at the library.

Thanks for patches and bug reports.

@qoobaa
Copy link
Owner

qoobaa commented Jul 15, 2011

Sure, that's enough. I didn't notice that pull request. Any tests for that?

@TelegramSam TelegramSam mentioned this issue Dec 16, 2011
@qoobaa qoobaa closed this as completed in 7cd400b Dec 16, 2011
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 a pull request may close this issue.

2 participants