Features and code reorganization to support the KBase Lakehouse - #243
jeff-cohere wants to merge 107 commits into
Conversation
… Globus share.
Specifically:
* I've added the ability to directly upload files to a Globus share via HTTPS.
* The logic governing Globus access keys has been simplified.
* The Root() method for the Endpoint interface has been broken into:
* a BasePath() method that returns the absolute path on the filesystem
below which files are not visible to a Globus share
* a DataPath() method that returns the path on the filesystem (relative to
BasePath()) where files of interest are located
Additionally, there are various small fixes and cleanups.
…user credentials. Work in progress.
|
|
The minio/minio Docker image seems unavailable as of a few minutes ago. Routine botstorm or discontinued? I suppose we'll find out soon. |
There was a problem hiding this comment.
🟡 Changes recommended
Cross-provider transfers currently contain blocking path, credential-handling, HTTP, concurrency, and secret-persistence defects.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds Lakehouse interoperability across local, Globus, and S3 endpoints.
Changes:
- Extends endpoint identity, path, and connection APIs.
- Adds Globus HTTPS and S3 credential integration via MMS.
- Introduces a KBase Lakehouse database stub and configuration updates.
File summaries
| File | Description |
|---|---|
transfers/transfers.go |
Updates provider and database registration. |
transfers/transfers_test.go |
Migrates endpoint path fixtures. |
transfers/store.go |
Simplifies transfer creation messages. |
transfers/mover.go |
Adds cross-provider connection setup. |
services/version.go |
Bumps version to 0.15.0. |
services/prototype.go |
Clarifies KBase authentication fallback. |
integration/irods/fixtures/test-config.yaml |
Migrates local path configuration. |
endpoints/s3/endpoint.go |
Implements expanded endpoint interface. |
endpoints/s3/endpoint_test.go |
Updates S3 path assertions. |
endpoints/local/endpoint.go |
Adds split paths and Globus HTTPS uploads. |
endpoints/local/endpoint_test.go |
Updates local endpoint fixtures. |
endpoints/globus/globus.go |
Adds dedicated Globus API clients. |
endpoints/globus/endpoint.go |
Refactors Globus endpoint integration. |
endpoints/endpoints.go |
Expands the endpoint contract. |
dtstest/dtstest.go |
Updates endpoint test doubles. |
deployment/dts.yaml |
Revises deployment endpoint configuration. |
databases/nmdc/database_test.go |
Migrates Globus path fixtures. |
databases/kbase_lakehouse/database.go |
Adds the Lakehouse database stub. |
auth/kbase_mms.go |
Adds MMS credential retrieval. |
auth/kbase_auth_server.go |
Associates users with connection credentials. |
auth/authenticator.go |
Initializes credential maps. |
auth/auth.go |
Extends users and credentials for connections. |
Review details
Suppressed comments (2)
endpoints/globus/globus.go:712
- As in
get,filepath.Joincorrupts an HTTPS URL (https://…becomeshttps:/…). This makes every Manager API POST, including user-credential registration, fail before reaching Globus.
resourcePath := filepath.Join(c.Url, resource)
endpoints/globus/endpoint.go:271
- This upload path ignores
DataPath, despite the endpoint contract defining it as the directory containing endpoint data. A configured destination such asglobus-kbase(data_path: jeff_cohere) receives local uploads under the base path instead. Apply the destination data path exactly once and reconcile the custom-destination path construction, which currently embeds that path inDestinationPath.
absPath := filepath.Join(ep.Paths.Base, resource)
- Files reviewed: 22/22 changed files
- Comments generated: 16
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
The DTS Globus/S3 integration isn't working yet. In order to transfer files from a Globus endpoint to an S3 endpoint, a user's S3 credentials must be registered with the storage gateway underlying the collection in which the files reside. During the registration process, the DTS presents the user's Globus ID, Globus username, and an S3 credentials policy containing credentials obtained from the KBase MMS (passed along via the store process, which records all information specific to a requested transfer when it's created. The error encountered is described in the code attempting the registration, and relevant output is dumped. There's a test script on and you can check the log messages in the DTS instance running in the Hopefully it won't take much more time/effort to take this to the finish line. I wish we had more time to get this working! |
This PR brings in some interoperability features needed to support a deployment of the DTS to the KBase Lakehouse environment:
Because we're now using several different Globus APIs, I've also reorganized the Globus logic into its own source file. This makes the Globus endpoint logic more transparent.
Closes #242