feat(storage): add support for partial list bucket #15763
Conversation
product-auto-label
bot
added
the
api: storage
bajajneha27
added
do not merge
product-auto-label
bot
added
the
api: storage
bajajneha27
force-pushed
the
401329823
branch
from
1a53d0c to
b023b7b
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #15763 +/- ##
==========================================
- Coverage 92.94% 92.94% -0.01%
==========================================
Files 2457 2458 +1
Lines 227084 227342 +258
==========================================
+ Hits 211066 211299 +233
- Misses 16018 16043 +25 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bajajneha27
force-pushed
the
401329823
branch
from
2d91f71 to
1e61e4d
Compare
bajajneha27
force-pushed
the
401329823
branch
from
75bad2b to
43611c3
Compare
bajajneha27
force-pushed
the
401329823
branch
from
43611c3 to
5d0d507
Compare
bajajneha27
removed
the
do not merge
|
|
||
| if (json.count("unreachable") != 0) { | ||
| for (auto const& kv : json["unreachable"].items()) { | ||
| result.unreachable.emplace_back(kv.value().get<std::string>()); |
There was a problem hiding this comment.
Should we use std::move() here?
There was a problem hiding this comment.
You mean on kv.value() ?
There was a problem hiding this comment.
The nlohmann_json library may limit the ability to move things. I'm pretty sure value() returns a copy of what the kv iterator is pointing at. However, as result.unreachable is a std::vector<std::string>, emplace_back isn't helping here, push_back(T&&) would be preferable as it will move whatever std::string the json library creates.
|
|
||
| std::vector<std::string> names; | ||
| std::vector<std::string> unreachable; | ||
| for (auto& r : client.ListBucketsExtended(ReturnPartialSuccess(true))) { |
There was a problem hiding this comment.
Do we need to add tests for the case where ReturnPartialSuccess is false or empty?
There was a problem hiding this comment.
By default it's false and we have existing testcase covering that.
If you think we should do it and we're doing it across languages then I can add.
|
|
||
| if (json.count("unreachable") != 0) { | ||
| for (auto const& kv : json["unreachable"].items()) { | ||
| result.unreachable.emplace_back(kv.value().get<std::string>()); |
There was a problem hiding this comment.
The nlohmann_json library may limit the ability to move things. I'm pretty sure value() returns a copy of what the kv iterator is pointing at. However, as result.unreachable is a std::vector<std::string>, emplace_back isn't helping here, push_back(T&&) would be preferable as it will move whatever std::string the json library creates.
4 participants
This change introduces support for partial list buckets in both gRPC and JSON transports.
We've introduced a new optional boolean flag,
returnPartialSuccess. By default, the flag isfalse.If the flag is set to
true, the API will return a list of buckets and will additionally populate a separate list of theunreachablebucket names.