-
-
Notifications
You must be signed in to change notification settings - Fork 427
Resolve multiple object names at once (with batching) #3398
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
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3398 +/- ##
==========================================
+ Coverage 70.50% 70.55% +0.04%
==========================================
Files 232 232
Lines 19999 20019 +20
==========================================
+ Hits 14101 14124 +23
+ Misses 5898 5895 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
011e4f4 to
649b8b2
Compare
da266a8 to
d1ac940
Compare
code style fixes
0db02cf to
d02d1c2
Compare
style fix
375a6fc to
ad6c612
Compare
bsipocz
left a comment
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.
I have one minor comment, otherwise it looks good.
There are also a bunch of super minor whitespace issues for the docs due to the changed printings, could you please fix those up, too?
| params[param_key] = items | ||
| resp = request_func(params) | ||
| resp.raise_for_status() | ||
| return extract_func(resp) |
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.
Shouldn't be the return value be a list in all cases?
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.
Changed this to always return a list of results for simplicity. In MastMissions.get_product_list, we only return the responses in a list if they are the product of batching.
ignore doctest outputs
0df1670 to
2c588eb
Compare
bsipocz
left a comment
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.
Thank you!
Support for resolving multiple object names at once with
resolve_object(), including automatic batching into groups of up to 30 names per request to the service.Some notes:
_batched_request. I also use this function for getting products viaMastMissions.get_product_list.resolve_allflag and whether or not multiple objects are being passed in. I detail input combinations and what they return in the function docstring and the documentation.