Update the create-and-run-flow example scripts - #1416
Conversation
The create-and-run-flow example was written under globus-sdk v3 , and used tokenstorage to handle logins. They have drifted out of date. The minimal fix would be to correct the import paths used, but these changes refit the scripts more dramatically to use `GlobusApp`. Not only are the scripts shorter and more to the point, with modernized usage, they are also fully type annotated, such that `tox r -e mypy-docs` passes on this part of the docs tree.
derek-globus
left a comment
derek-globus
left a comment
There was a problem hiding this comment.
One minor suggestion but otherwise looks good; good improvement to use GlobusApp & the use of sweep makes sense here.
| def get_flows_client(app: globus_sdk.GlobusApp) -> globus_sdk.FlowsClient: | ||
| return globus_sdk.FlowsClient( | ||
| app=app, app_scopes=[globus_sdk.FlowsClient.scopes.manage_flows] | ||
| ) | ||
|
|
||
|
|
||
| def create_flow(args): | ||
| flows_client = get_flows_client() | ||
| print( | ||
| flows_client.create_flow( | ||
| title=args.title, | ||
| definition={ | ||
| "StartAt": "DoIt", | ||
| "States": { | ||
| "DoIt": { | ||
| "Type": "Action", | ||
| "ActionUrl": "https://actions.globus.org/hello_world", | ||
| "Parameters": { | ||
| "echo_string": "Hello, Asynchronous World!", | ||
| }, | ||
| "End": True, | ||
| } | ||
| def create_flow(app: globus_sdk.GlobusApp, args: argparse.Namespace) -> None: | ||
| with get_flows_client(app) as flows_client: |
There was a problem hiding this comment.
What do you think of removing the get_flows_client call entirely?
There's not too much benefit in scoping down the request space from flows:all -> flows:manage_flows, they're both very expansive in terms of what they allow.
At that point, with globus_sdk.FlowsClient(app=app) is basically the same length as with get_flows_client(app) without hiding the call behind a function.
There was a problem hiding this comment.
Oh, is our default all?
I tried it with the call inlined, but then I found that it was harder to point out "here's where and how we construct the client, with configured scopes". If we don't need to configure scopes I'm in favor of inlining it. Let me check/confirm and apply.
There was a problem hiding this comment.
Oh, is our default all?
Because the `all` scope is the default, inlining improves readability. Co-authored-by: derek-globus <113056046+derek-globus@users.noreply.github.com>
2 participants
The create-and-run-flow example was written under globus-sdk v3 , and used tokenstorage to handle logins. They have drifted out of date.
The minimal fix would be to correct the import paths used, but these changes refit the scripts more dramatically to use
GlobusApp.Not only are the scripts shorter and more to the point, with modernized usage, they are also fully type annotated, such that
tox r -e mypy-docspasses on this part of the docs tree.@derek-globus, for your particular interest, note the use of
logout(sweep=True)in these examples. 😁