Skip to content

feature/globus - #251

Open
bwalsh wants to merge 15 commits into
developmentfrom
feature/globus
Open

feature/globus#251
bwalsh wants to merge 15 commits into
developmentfrom
feature/globus

Conversation

@bwalsh

@bwalsh bwalsh commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds Globus Transfer support for hydrating DRS objects into the Git LFS cache.

  • Selects usable Globus access methods while falling back per object when unavailable.
  • Resolves DRS access_id values correctly.
  • Authenticates with the Globus Transfer API.
  • Submits checksum-synchronized transfers using valid Transfer API payloads.
  • Writes directly to the repository’s .git/lfs/objects cache path.
  • Polls recoverable INACTIVE tasks until completion or cancellation.
  • Verifies downloaded size and SHA-256 before exposing cached content.
  • Rejects unsupported Globus remote-auth configuration.
  • Updates user and developer documentation.

Configuration

The destination Globus collection must expose the repository root.

  export GIT_DRS_GLOBUS_TRANSFER_TOKEN='<token>'
  export GIT_DRS_GLOBUS_DESTINATION_COLLECTION='<collection-id>'

To prefer Globus when available:

  export GIT_DRS_ACCESS_METHOD=globus

Testing

  go test ./...

All tests pass.

Reviewer note

Please pay particular attention to the documentation changes:

  • ADR: docs/access-method-selection-and-authentication.md

    • Per-object access-method selection
    • Preference and fallback behavior
    • Separation of transfer selection from authentication
    • Mixed Globus and non-Globus pulls
  • Developer architecture: docs/globus-notes-high-level.md

    • Globus ownership boundaries
    • Destination collection and LFS cache mapping
    • Client-managed transfer workflow
  • User documentation: docs/globus.md

    • Required environment variables
    • Destination collection setup
    • Globus preference behavior
    • Authentication and troubleshooting
  • Troubleshooting: docs/troubleshooting.md

    • Globus token and collection checks
    • LFS cache visibility requirements

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f8af218063

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/transfer/download.go Outdated
Comment on lines +74 to +75
if method.AccessUrl != nil && strings.TrimSpace(method.AccessUrl.Url) != "" {
return &drsapi.AccessURL{Headers: method.AccessUrl.Headers, Url: strings.TrimSpace(method.AccessUrl.Url)}, nil

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep resolving provider access URLs through DRS

When a record was created by add-url/push with an inline provider URL, this branch returns the raw s3:///gs:// value stored in AccessMethod.AccessUrl (see internal/drsobject/object.go:125-130 and docs/adding-s3-files.md:7-11). DownloadResolvedToPath then hands that URL to Syfon's GenericDownload, whose implementation only treats file paths specially and otherwise performs an HTTP GET (https://github.com/calypr/syfon/blob/development/client/transfer/http.go#L676-L804), so normal provider-backed pulls fail with an unsupported protocol scheme instead of asking the DRS server for a presigned URL. Restrict direct returns to schemes the downloader handles, or continue resolving storage-provider URLs through DRS.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@bwalsh

bwalsh commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

@chatgpt-codex-connector review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fe920e3e84

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/transfer/download.go Outdated
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.

1 participant