Add support for device auth flow#100
Conversation
| matplotlib | ||
| pandas | ||
|
|
||
| git+https://github.com/ChameleonCloud/ccauth.git |
msherman64
left a comment
There was a problem hiding this comment.
It all seems like it works to me, just some nits based on how we package it.
changing python versions and setting the project domain seem fine, but I feel like they should be top level changes instead of bundled into the device auth change.
for the device auth, we'll need to decide whether cc-auth is a hard dependency+import, or optional, the way python-blazarclient is used.
and finally, how are things imported? should use_device_auth be added to:
init.py's from .context import get, params, reset, session, set, use_site?
| runs-on: ubuntu-24.04 | ||
| strategy: | ||
| matrix: | ||
| python: | ||
| - 3.8 | ||
| - "3.10" | ||
| - "3.11" | ||
| - "3.12" | ||
| - "3.13" |
There was a problem hiding this comment.
is this bump because cc-auth doesn't support python3.8?
I would leave this as a separate commit/pr and update tox.ini as well to match, just to make it clear that this is a "deprecate python 3.8 and 3.9, test 3.10 to 3.13" change
There was a problem hiding this comment.
I'll leave this in the PR, but it wasn't ccauth specifically. I think it was setuptools in general was not playing nice in GHA.
| set( | ||
| "project_domain_name", DEFAULT_PROJECT_DOMAIN_NAME | ||
| ) # Same for all chameleon sites |
There was a problem hiding this comment.
if this is always setting the project domain name in the global context, should it be pulled out to a separate change/fix? I know it came up when using an external session from openstacksdk
| "/.well-known/openid-configuration" | ||
| ) | ||
| DEFAULT_PROTOCOL = "openid" | ||
| DEFAULT_RESOURCE_PROVIDER = "chameleon" |
There was a problem hiding this comment.
should this be named DEFAULT_IDENTITY_PROVIDER ?
This changes makes it so if a user enables device auth flow with
chi.context.use_device_auth()that will be used when setting up the python-chi session.