Unify EGI and Globus Authorization - #52
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the transaction API to use a single shared Authorizer implementation from esgf-core-utils across deployments, and improves error handling around extension schema retrieval and STAC validation feedback.
Changes:
- Replaced deployment-specific authorization branching with the shared
esgf_core_utils.models.auth.Authorizer. - Added more detailed error handling when downloading/parsing extension schemas.
- Updated default extension schema URLs and bumped
esgf-core-utilsdependency.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils.py | Adds HTTP/error/JSON parsing handling for extension schema downloads; enriches validation error details. |
| src/settings/init.py | Updates default extension schema URLs/versions used for validation. |
| src/client.py | Switches transaction authorization to the shared esgf-core-utils Authorizer. |
| src/authorizer/globus_authorizer.py | Migrates Globus middleware context to build the shared esgf-core-utils Authorizer; adds S3 policy loading support. |
| src/authorizer/globus_auth.py | Removes the repo-local GlobusAuth implementation in favor of the shared Authorizer. |
| poetry.lock | Bumps esgf-core-utils to 1.1.0 (and related locked dependency metadata). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| response = httpx.get(extension) | ||
| response.raise_for_status() | ||
| schema = response.json() |
| except httpx.HTTPStatusError as exc: | ||
| e = UnexpectedExtensionException(extension=extension) | ||
| e.detail = f"Error {exc.response.status_code} while getting the extension schema {exc.request.url}" | ||
| raise e |
| except httpx.RequestError as exc: | ||
| e = UnexpectedExtensionException(extension=extension) | ||
| e.detail = f"An error occured while getting the extension schema {exc.request.url}" | ||
| raise e |
| except json.JSONDecodeError as exc: | ||
| e = UnexpectedExtensionException(extension=extension) | ||
| e.detail = f"Failed to decode the extension schema {extension}: {exc.msg}" f"Error occured at line: {exc.lineno}, column: {exc.colno}" | ||
| raise e |
|
Very minor concern. There are now two things named I understand one is middleware used in the fastapi app and the other exposes a pydantic model with various methods. Just wanted to call this out :) |
sturoscy-personal
left a comment
sturoscy-personal
left a comment
There was a problem hiding this comment.
Left a quick comment in the PR, but this otherwise looks fine to me.
The PR does update two things: