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

Globus app configurable environment #1001

Merged

Conversation

derek-globus
Copy link
Contributor

@derek-globus derek-globus commented Jul 11, 2024

Added GlobusAppConfig.environment.

If omitted, this value is set by looking up the GLOBUS_SDK_ENVIRONMENT variable, defaulting to "production" if unset.


Notable changes:

(A) A client's environment inference heirarchy is now:

  • If GlobusApp is supplied, the client's environment must either match the app's or be omitted
    • GlobusApp environment can be supplied explicitly via GlobusAppConfig or is read from the GLOBUS_SDK_ENVIRONMENT environment variable.
  • Otherwise:
    • The explicitly provided environment to the client constructor
    • The environment variable GLOBUS_SDK_ENVIRONMENT
    • The default (production)

(B) GlobusApp default token storage is now prefixed with the environment name for lower envs.
So for example, in the following snippet

config1 = GlobusAppConfig(environment="production")
app1 = UserApp("my-cool-app", client_id=..., config=config1)

config2 = GlobusAppConfig(environment="sandbox")
app2 = UserApp("my-cool-app", client_id=..., config=config2)

app1's tokens would be stored at ~/.globus/app/my-cool-app/tokens.json
app2's tokens would be stored at ~/.globus/app/my-cool-app/sandbox-tokens.json


📚 Documentation preview 📚: https://globus-sdk-python--1001.org.readthedocs.build/en/1001/

@derek-globus
Copy link
Contributor Author

Discussed this PR offline w/ Kurt.

Ended up modifying it in a couple ways:

  1. LoginClients created while instantiating a GlobusApp use the configured environment (this I meant to do but missed).
  2. Supplied LoginClients must match the GlobusApp's configured environment (otherwise a GlobusSDKUsageError is raised on GlobusApp instantiation)
  3. Clients with an attached GlobusApp must use the same environment as their attached GlobusApp. See below for valid and invalid usages.
sandbox_app = UserApp("my-app", client_id=..., config=GlobusAppConfig(environment="sandbox"))

# Valid - instantiates a sandbox flows client
FlowsClient(app=sandbox_app)
# Also valid - a little redundant but nothing wrong here
FlowsClient(app=sandbox_app, environment="sandbox")

# Invalid - Raises GlobusSDKUsageError
FlowsClient(app=sandbox_app, environment="preview")

and the (probably) more contentious case

# Environment is inferred to default "production" environment
production_app = UserApp("my-app", client_id=...)

# Invalid - app inferred "production" but client specified "sandbox"
FlowsClient(app=production_app, environment="sandbox")

@aaschaer
Copy link
Contributor

1. LoginClients created while instantiating a GlobusApp use the configured environment (this I meant to do but missed).

Good catch @kurtmckee!

2. Supplied LoginClients must match the GlobusApp's configured environment (otherwise a GlobusSDKUsageError is raised on GlobusApp instantiation)

3. Clients with an attached GlobusApp must use the same environment as their attached GlobusApp. See below for valid and invalid usages.

I think I slightly prefer this over the hierarchy, as I feel like I'm more likely to miss a mismatch than need to run a script that touches multiple environments since all our current tooling assumes one environment per run. Don't think it matters too much one way or the other though since this will probably be entirely internal

@derek-globus
Copy link
Contributor Author

I feel like I'm more likely to miss a mismatch than need to run a script that touches multiple environments since all our current tooling assumes one environment per run

In the case that you do need to touch multiple environments, you still can! You'll just need to make two GlobusApps explicitly.

@derek-globus derek-globus merged commit acc8cc9 into globus:main Jul 12, 2024
15 checks passed
@derek-globus derek-globus deleted the globus-app-configurable-environment branch July 12, 2024 17:14
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.

3 participants