Add globus api gcs command for direct GCS Manager API usage
#913
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
The addition of this command requires some large-diff-but-small-logic changes to the `globus api` command definition module. Firstly, the major logical components of these commands are separated into three helpers: - a decorator for the common parameters - an "execute service command" helper - a "handle scope string" helper (more on why this is needed below) The command callback passes all of its parameters verbatim to the execution helper, so there's very little change to those parts of the command. The execution helper consumes any object of type `globus_sdk.BaseClient`, meaning it can be passed any service client, already authenticated/configured, and it uses that object equivalently. The "handle scope string" component needs to be treated differently because it executes *before* there is a client object in hand, and alters how that client object is authenticated for later use. It differs subtly between the known services (handled by name) and GCS (where `resource_server == endpoint_id`). The GCS variant uses a positional arg for ENDPOINT_ID, so it is declared separately from the other commands. Effectively, it's a different interface wrapping the same functionality.
sirosen
requested review from
aaschaer,
ada-globus,
derek-globus and
kurtmckee
as code owners
kurtmckee
approved these changes
Dec 9, 2023
Co-authored-by: Kurt McKee <[email protected]>
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.
The addition of this command requires some large-diff-but-small-logic changes to the
globus apicommand definition module.Firstly, the major logical components of these commands are separated into three helpers:
The command callback passes all of its parameters verbatim to the execution helper, so there's very little change to those parts of the command. The execution helper consumes any object of type
globus_sdk.BaseClient, meaning it can be passed any service client, already authenticated/configured, and it uses that object equivalently.The "handle scope string" component needs to be treated differently because it executes before there is a client object in hand, and alters how that client object is authenticated for later use. It differs subtly between the known services (handled by name) and GCS (where
resource_server == endpoint_id).The GCS variant uses a positional arg for ENDPOINT_ID, so it is declared separately from the other commands. Effectively, it's a different interface wrapping the same functionality.
One thing I noticed while testing was that we're currently doing some name-shadowing (purely accidentally) of
service_name, which gets defined in module-scope for use in a loop. In my first draft, this caused a bug asservice_namewas defined, but didn't have the value I expected. I'd like to fix this in any of a number of ways, but I've kept it out of scope for this PR.