Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Client copy - Issue 670 #671

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

b-eisen
Copy link

@b-eisen b-eisen commented Oct 2, 2023

This code implements the feature request in #670. The code implements a number of new flags specific to the CP command to allow for copying from an external S3 compatible API to an S3 bucket in region. The implementation takes the existing download and upload methods, calling them in one function and saving, then deleting a file which is temporary on the local filesystem.

                &cli.BoolFlag{
			Name:    "client-copy",
			Aliases: []string{"cc"},
			Usage:   "copies from S3 bucket to S3 bucket through the local client (Advanced Use Case)",
		},
		&cli.StringFlag{
			Name:  "source-region-profile",
			Usage: "use the specified profile from the credentials file for the source region",
		},
		&cli.StringFlag{
			Name:  "destination-region-profile",
			Usage: "use the specified profile from the credentials file for the destination region",
		},
		&cli.StringFlag{
			Name:    "source-region-endpoint-url",
			Usage:   "override default S3 host for custom services for the source region",
			EnvVars: []string{"S3_ENDPOINT_URL"},
		},
		&cli.BoolFlag{
			Name:  "source-region-no-verify-ssl",
			Usage: "disable SSL certificate verification for the source region endpoint",
		},
		&cli.StringFlag{
			Name:    "destination-region-endpoint-url",
			Usage:   "override default S3 host for custom services for the destination region",
			EnvVars: []string{"S3_ENDPOINT_URL"},
		},
		&cli.BoolFlag{
			Name:  "destination-region-no-verify-ssl",
			Usage: "disable SSL certificate verification for the destination region endpoint",
		},

New E2E tests have been and all tests pass (except for TestRunSpecialCharactersInPrefix in run_test.go) which failed before new code was written. I have also performed integration testing on MacOS, Linux and Windows with both minio and S3 compatible API on Snowball Edge devices.

Make all fails on my local setup due to an issue with staticcheck. VSCode plugin shows no linting errors. It failed with

make: staticcheck: No such file or directory
make: *** [staticcheck] Error 1

@b-eisen b-eisen requested a review from a team as a code owner October 2, 2023 18:24
@b-eisen b-eisen requested review from igungor and denizsurmeli and removed request for a team October 2, 2023 18:24
@b-eisen b-eisen changed the title FeatL: Client copy - Issue 670 feat: Client copy - Issue 670 Oct 2, 2023
@b-eisen
Copy link
Author

b-eisen commented Oct 4, 2023

@igungor @denizsurmeli - Good day. Following up to see if you have had a chance to look or have any questions on this PR. Thank you.

@b-eisen
Copy link
Author

b-eisen commented Oct 10, 2023

@igungor @denizsurmeli - Good day again. Following up to see if you have had a chance to look at this PR or have any questions. Thank you.

@denizsurmeli
Copy link
Contributor

Hi, thanks for your work. This is a issue often mentioned for s5cmd. I would like to share my two cents about it, but obviously we can and will discuss with the team members about this feature. s5cmd is basically a file system tool. It's designed to operate on one filesystem. The feature you proposing is somehow using the host tool as a bridge, to sync two 'file systems'. There are separate tools that just designed for that purpose, such as rclone. This feature feels like out of the purpose of s5cmd, but again, this is a personal comment. Again, thanks for your contribution, we appreciate your work.

@b-eisen
Copy link
Author

b-eisen commented Oct 11, 2023

Thank you for your response. I understand that point of view and that it may not align with the purpose of s5cmd. I will note, however, that in my course of work I have been made away of users who want to continue to take advantage of the high performant nature of s5cmd while working with multiple s3 compatible endpoints. They have made workarounds that essentially wrap s5cmd in other tools (python scripts, etc.). This would allow users to have additional capability without needing the complexity of additional scripts, tools, etc.

In my firsthand experiences those users have looked at tools like rclone, but prefer the performance and familiarity of s5cmd.

Thank you for your consideration.

@b-eisen
Copy link
Author

b-eisen commented Oct 28, 2023

Good day. Following up to see if the team has discussed this issue yet?

@k0ste
Copy link

k0ste commented Nov 22, 2023

Actually it will be nice to see ability of s5cmd to work with two s3 endpoints. For example, when you need "add another region" (cp region-1 to new region-2) or move images from one region to another

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.

None yet

3 participants