Skip to content
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

Revise Example Timer Script #672

Merged
merged 2 commits into from
Jan 25, 2023
Merged

Revise Example Timer Script #672

merged 2 commits into from
Jan 25, 2023

Conversation

ada-globus
Copy link
Contributor

dsc_3114_jt-1

Description

Makes some changes to the example Timer script based on discoveries while supporting a user that was running into trouble getting this running:

  • Instantiate the native client
  • Handle mapped collections
  • Remove Transfer client from TransferData invocation
  • Use Transfer action provider in scope tree and revise dependencies
  • Specifically send the constructed Transfer AP scope

Copy link
Member

@sirosen sirosen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's a small missing piece, and then there are two discussion points for us to address.

The only one which must be addressed, as far as I'm concerned, is the data = ... one.

transfer_authorizer = AccessTokenAuthorizer(transfer_token)
transfer_client = TransferClient(authorizer=transfer_authorizer)
data = TransferData(transfer_client, GOEP1, GOEP2)
data.add_item("/share/godata/file1.txt", "/~/file1.txt")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing this removal, but not the redefinition of data. Do you have an unpushed change for this, perhaps?

I'm guessing that we just want

data = TransferData(source_endpoint=GOEP1, destination_endpoint=GOEP2)
data.add_item(...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep—this was missed in a a copy-paste error when I was moving bits from the script I wrote for the support ticket! 🤦‍♀️

docs/examples/timer_operations.rst Outdated Show resolved Hide resolved
interval,
stop_after_n=2,
name=name,
scope=str(transfer_action_provider_scope),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify, this is passing the string-ized full dependency list.
Is it also valid to pass Timer only this?
scope="https://auth.globus.org/scopes/actions.globus.org/transfer/transfer"

It seems to me that if Timer is going to take its token, do a dependent token grant, and then lookup values in the response, we really ought to be passing it the resource server name ("actions.globus.org", I think?) rather than a scope. Or is there some other purpose here, which requires the full scope be passed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good call—I had that wrong!

@ada-globus ada-globus force-pushed the an/revise-timer-example branch from 845064c to 9279f72 Compare January 25, 2023 05:50
Changes:
- Instantiate the native client
- Handle mapped collections
- Remove Transfer client from TransferData invocation
- Use Transfer action provider in scope tree and revise dependencies

Signed-off-by: Ada <ada@globus.org>
@ada-globus ada-globus force-pushed the an/revise-timer-example branch from 9279f72 to 9e97fd4 Compare January 25, 2023 05:56
Signed-off-by: Ada <ada@globus.org>
@ada-globus ada-globus force-pushed the an/revise-timer-example branch from be150ce to 8c3ba30 Compare January 25, 2023 05:58
Copy link
Member

@sirosen sirosen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this!

I'm going to merge right away. You'll see it publish to the latest docs as soon as RTD does the build, but it won't reach stable until we do a release.

@sirosen sirosen merged commit b114bb2 into main Jan 25, 2023
@sirosen sirosen deleted the an/revise-timer-example branch January 25, 2023 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants