Skip to content

Conversation

@lbeckman314
Copy link
Contributor

@lbeckman314 lbeckman314 commented Oct 8, 2025

Overview 🌱

This PR reverts changes to OHSU's forked version of gen3-client in order to fix compilation issues related to the unbound furObjects variable.

This PR will make our master branch identical to that of upstream's (including the updates from uc-cdis/cdis-data-client#123 required to run multipart uploads to non-AWS S3 buckets).

Important

This PR also resolves CALYPR Issue #6 related to the misleading version message being included in the command output

Previous Behavior ❌

Compile errors:

➜ go build -o cdis-data-client
# github.com/uc-cdis/gen3-client/gen3-client/g3cmd
gen3-client/g3cmd/upload-multiple.go:82:30: undefined: furObjects
gen3-client/g3cmd/upload-multiple.go:84:13: undefined: batchFURObjects
gen3-client/g3cmd/upload-multiple.go:84:32: undefined: workers
gen3-client/g3cmd/upload-multiple.go:85:32: undefined: batchFURObjects
gen3-client/g3cmd/upload-multiple.go:87:34: undefined: batchFURObjects
gen3-client/g3cmd/upload-multiple.go:87:51: undefined: workers
gen3-client/g3cmd/upload-multiple.go:87:60: undefined: respCh
gen3-client/g3cmd/upload-multiple.go:87:68: undefined: errCh
gen3-client/g3cmd/upload-multiple.go:88:7: undefined: batchFURObjects
gen3-client/g3cmd/upload-multiple.go:89:32: undefined: batchFURObjects
gen3-client/g3cmd/upload-multiple.go:89:32: too many errors

Sync 🌀

Tip

Upstream Commit b9b3605

➜ git clone git@github.com:calypr/data-client.git
Cloning into 'data-client'...

➜ cd data-client

git show --oneline
a66704f (HEAD -> master, origin/master, origin/HEAD) Merge branch 'uc-cdis:master' into master

➜ git remote add upstream https://github.com/uc-cdis/cdis-data-client

➜ git fetch --all
Fetching origin
Fetching upstream

➜ git cherry-pick b9b3605 -X theirs
Auto-merging gen3-client/g3cmd/upload-multiple.go
Auto-merging gen3-client/g3cmd/upload-single.go
Auto-merging gen3-client/g3cmd/upload.go
Auto-merging gen3-client/g3cmd/utils.go
[fix/compile 3cab9cb] (PPS-411): Add bucket selection support for multi part upload (#123)
 Author: Binam Bajracharya <44302895+BinamB@users.noreply.github.com>
 Date: Mon Jan 22 12:59:20 2024 -0500
 4 files changed, 13 insertions(+), 92 deletions(-)

➜ git push
To github.com:calypr/data-client.git
 * [new branch]      fix/compile -> fix/compile
branch '[fix/compile](https://github.com/calypr/data-client/tree/fix/compile)' set up to track 'origin/fix/compile'.

New Behavior ✅

Successful compilation!

➜ go build -o cdis-data-client

➜ ls cdis-data-client
cdis-data-client

➜ ./cdis-data-client
Gen3 Client for downloading, uploading and submitting data to data commons.
gen3-client version: N/A, commit: N/A

Test Steps ✍️

Adapted from the Download and Upload Files Using the Gen3-client and Upload Data Files using the Gen3 Client

1. Build gen3-client

➜ which gen3-client
~/.gen3/gen3-client

➜ mv ~/.gen3/gen3-client ~/.gen3/gen3-client.bak

➜ go build -o ~/.gen3/gen3-client

➜ gen3-client --version
gen3-client version N/A

2. Upload Single File via gen3-client

echo "This is a test file for gen3-client" > gen3-client-test.txt

➜ gen3-client upload --profile=cbds --upload-path=gen3-client-test.txt
  • Verify that the gen3-client-test.txt file has been uploaded to the expected bucket

3. Upload Single File via calypr_admin

➜ calypr_admin clone cbds-dev

➜ cd cbds-dev

➜ echo "This is a test file for calypr_admin" > calypr_admin-test.txt

➜ calypr_admin add calypr_admin-test.txt

➜ calypr_admin meta init

➜ calypr_admin commit -m "adding test file"

➜ calypr_admin push

# Then verify that the `calypr_admin-test.txt` file has been uploaded to the expected bucket 

Next Steps 🌀

Copilot AI review requested due to automatic review settings October 8, 2025 19:13
@lbeckman314 lbeckman314 self-assigned this Oct 8, 2025
@lbeckman314 lbeckman314 added the bug Something isn't working label Oct 8, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR synchronizes the codebase with upstream/master commit b9b3605, primarily focusing on refactoring upload functionality and improving code organization. The changes involve extracting common multipart upload logic into a reusable function and removing unused validation code.

  • Extracted multipart upload logic into a dedicated processMultipartUpload function
  • Removed unused validateObject function from utils
  • Updated command flag descriptions to clarify default bucket behavior

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
gen3-client/g3cmd/utils.go Removed unused validateObject function
gen3-client/g3cmd/upload.go Refactored multipart upload logic to use new processMultipartUpload function and updated bucket flag description
gen3-client/g3cmd/upload-single.go Updated bucket flag description for consistency
gen3-client/g3cmd/upload-multiple.go Simplified file path processing logic, updated bucket flag description, and added new command flags
gen3-client/g3cmd/gitversion.go Changed variable declarations from var to const and reset version to "N/A"

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

* Add bucket param for multipart upload

* Resolve comments

* Add bucket param for single file upload

* add support for upload-single

* Get upload-multiple + multipart working

* Fix small files not working with multipart

* Fix upload

* chore(comment): insignificant commit to force test reruns

* Refactor upload-multiple

* Use file_name provided in manifest (#125)

* Use file_name provided in manifest

* Use file_name provided in manifest

* Ensure file_name used in url

* Ensure bucket passed to GeneratePresignedURL

* Fix batch

* fix utils

* fix utils.go

* cleanup+remove validateFilePath

---------

Co-authored-by: Alexander VanTol <Avantol13@users.noreply.github.com>
Co-authored-by: Brian <brian@bwalsh.com>
@lbeckman314 lbeckman314 merged commit 55ab900 into master Oct 27, 2025
6 of 10 checks passed
@lbeckman314 lbeckman314 deleted the fix/compile branch October 27, 2025 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

furObjects undefined? gen3-client: Remove latest version message from output

3 participants