Add 'globus api' command #626
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sirosen
commented
Apr 14, 2022
|
I've just pushed an intentionally-failing commit which I'll squash/amend later. It serves to explain some of the work I'm about to open in a PR against globus-sdk. |
This is lacking in testing, but works under some simple example usages. There's no detailed documentation and several parameters need more docs.
- Support headers - Improve content-type detection - Add '--body-file' as mutex with '--body' - Set "raw-api-command" in app_name on clients - Improve --help output
`for x in ...` binds `x` dynamically inside the loop scope, and we wanted to have a closure over `x`. An explicit function to act as a closure avoids this sabotaging the `globus api` command. This was discovered with the addition of a variety of unit tests and a functional test for the `globus api` command.
Allow error responses to be rendered verbatim (without our handlers running) if `--allow-errors` is used. Do not follow redirects unless `--allow-redirects/--location/-L` is used. The new attributes of response and error classes make it easy to unify the printing logic such that errors and responses are printed in the same way. The only difference between the two is the name of the text body attribute.
`globus api` commands now allow a flag `--no-retry` to disable automatic request retries. The tests for `globus api` which use error responses now pass `--no-retry`.
Instead of ": ", split on ":" and trim exactly one leading whitespace if present. This behavior is probably simpler for users and can still express any amount of whitespace in a header.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Right now, this is just a "first cut", and needs a bit more work. However, I was able to use it to do a few Transfer calls and it seems okay.
This is lacking in testing, there's no detailed documentation, and several parameters need more docs.
In no particular order, these are the TODO items which I still have on this:
--body-file, make it mutex with--bodydata: strfor request bodies--headeroption withmultiple=Trueapp_nameon clients in the api commands to"globus-cli-raw"+ CLI version infoUpdate: This is now ready for review.
It add
globus api [transfer|auth|groups|search] [HEAD|GET|PUT|POST|DELETE] PATH, with options for setting headers and query params with some basic parsing.Some specifics on
Content-Type:Because many Globus APIs expect or require a content-type when reading JSON data, it seemed worthwhile to add some special handling for it. Namely, there is some simple Content-Type detection which will treat json-parseable data as
Content-Type: application/json, and this can be controlled or fine-tuned with--content-type. It is applied to the headers before--headervalues are parsed and handled, so that--header "Content-Type: application/octet-stream"works as desired/expected and overrides the autodetect behavior.