-
Notifications
You must be signed in to change notification settings - Fork 239
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
Add a BaseBlobService.list_blob_names method #545
Conversation
This can currently also be achieved via `[b.name for b in bbs.list_blobs()]` but that parses the full XML and discards most of the parsed information again. With this change listing the blob names is not anymore CPU-bound for us.
Travis fails as the casette in the tests needs to be recorded. Any advice on this? |
Hi @xhochy, thanks for the PR! The additional method seems to be very specific to your use case. Is it possible to keep it in your application code instead? |
I can keep that in my application code but I made a PR here as I don't think that it is specific for my use case but rather generally useful. When working with object stores, often people are only interested in the filenames and not any additional information. The additional information information is often not required but in my measurements, a request for 5000 keys took 20-30s whereof 3s were for the request itself and the remaining time was spent on parsing the XML and constructing the I would use the code in this PR in https://github.com/mbr/simplekv and an ABS-compatible version of https://github.com/martindurant/filesystem_spec / https://github.com/dask/s3fs for |
I've opened a PR over at mbr/simplekv#88 to use the code directly instead of changing anything in |
Hi @xhochy, thanks for getting back to me. I see your point, and tend to agree with you. But let me talk to the Team first, to make sure. In the mean time, could you please run the test in record mode? To generate the recording file. |
I would like to back this request. We also have a need for just filenames. |
@seguler what do you think? |
@seguler @zezha-msft Any updates in whether this can go in? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me.
Is there a plan or schedule on when this PR will be included in a release? |
Hi @BY-jk, I was planning on making a release by the end of this week. Is there a timeline requirement? |
@zezha-msft - end of this week would be great. While we do have workarounds in place, this will allow for a number of down-stream libraries to avoid adding more custom code. Thanks for getting back and pushing this forward. Very much appreciated. |
@xhochy @BY-jk this change was just released. Thanks for your patience! |
This can currently also be achieved via
[b.name for b in bbs.list_blobs()]
but that parses the full XML and discards most of the parsed information again. With this change listing the blob names is not anymore CPU-bound for us.