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

Error in sync command when using --delete flag with 2.1.0 release #576

Closed
jsproede opened this issue Jun 21, 2023 · 2 comments · Fixed by #578
Closed

Error in sync command when using --delete flag with 2.1.0 release #576

jsproede opened this issue Jun 21, 2023 · 2 comments · Fixed by #578

Comments

@jsproede
Copy link

Hi 👋

Firstly I want to thank you for this great tool!

After updating from 2.1.0-beta.1 to 2.1.0 we've encountered a strange behavior when syncing a local folder to our S3 buckets. We're not using AWS, but I could reproduce this issue with a LocalStack S3 bucket locally.

Using s5cmd in version 2.1.0-beta.1 syncs a local folder without any issues:

❯ ls -l
total 0
-rw-r--r--  1 x  staff  0 21 Jun 07:47 Test1.txt
-rw-r--r--  1 x  staff  0 21 Jun 07:47 Test2.txt
❯ s5cmd-2.1.0-beta version
v2.1.0-beta.1-3e08061
❯ s5cmd-2.1.0-beta ls s3://test-bucket/\*
ERROR "ls s3://test-bucket/*": no object found
❯ s5cmd-2.1.0-beta --stat --log debug sync --delete ./ s3://test-bucket/
cp Test2.txt s3://test-bucket/Test2.txt
cp Test1.txt s3://test-bucket/Test1.txt

Operation	Total	Error	Success
sync		1	0	1
cp		2	0	2

❯ s5cmd-2.1.0-beta ls s3://test-bucket/\*
2023/06/21 05:54:06                 0 Test1.txt
2023/06/21 05:54:06                 0 Test2.txt
❯ s5cmd-2.1.0-beta --stat --log debug sync --delete ./ s3://test-bucket/
DEBUG "sync Test1.txt s3://test-bucket/Test1.txt": object is newer or same age and object size matches
DEBUG "sync Test2.txt s3://test-bucket/Test2.txt": object is newer or same age and object size matches

Operation	Total	Error	Success
sync		1	0	1

Running the exact same commands with s5cmd in version 2.1.0 results in errors (ERROR "rm --raw=true": expected at least 1 object to remove) when using the --delete flag with the sync command:

❯ ls -l
total 0
-rw-r--r--  1 x  staff  0 21 Jun 07:47 Test1.txt
-rw-r--r--  1 x  staff  0 21 Jun 07:47 Test2.txt
❯ s5cmd version
v2.1.0-3efbbe8
❯ s5cmd ls s3://test-bucket/\*
ERROR "ls s3://test-bucket/*": no object found
❯ s5cmd --stat --log debug sync --delete ./ s3://test-bucket/
ERROR "rm --raw=true": expected at least 1 object to remove
cp Test2.txt s3://test-bucket/Test2.txt
cp Test1.txt s3://test-bucket/Test1.txt

Operation	Total	Error	Success
cp		2	0	2
sync		1	1	0

❯ s5cmd ls s3://test-bucket/\*
2023/06/21 05:49:47                 0  Test1.txt
2023/06/21 05:49:47                 0  Test2.txt
❯ s5cmd --stat --log debug sync --delete ./ s3://test-bucket/
DEBUG "sync Test1.txt s3://test-bucket/Test1.txt": object is newer or same age and object size matches
DEBUG "sync Test2.txt s3://test-bucket/Test2.txt": object is newer or same age and object size matches
ERROR "rm --raw=true": expected at least 1 object to remove

Operation	Total	Error	Success
sync		1	1	0

Is there something we need to change in CLI usage after updating to version 2.1.0? I don't see any changes regarding the sync command and --delete flag in the release notes. As the sync command is now failing we cannot update s5cmd in our CI/CD pipelines.

If you need anything further, feel free to let me know.

Thank you!

@kucukaslan
Copy link
Contributor

kucukaslan commented Jun 21, 2023

If understood correctly there is nothing to remove nevertheless sync tries to create a rm command.

If that is the case it might be related to #483

s5cmd/command/sync.go

Lines 462 to 477 in fa8f356

// only in destination
if s.delete {
// unfortunately we need to read them all!
// or rewrite generateCommand function?
dstURLs := make([]*url.URL, 0, extsortChunkSize)
for d := range onlyDest {
dstURLs = append(dstURLs, d)
}
command, err := generateCommand(c, "rm", defaultFlags, dstURLs...)
if err != nil {
printDebug(s.op, err, dstURLs...)
return
}
fmt.Fprintln(w, command)
} else {

I guess I missed to check whether dstURLs is empty unlike the previous version of code1:

if s.delete && len(onlyDest) > 0 {

ps. @peak/big-data2 you may want to include this in milestone:v2.2.0

Footnotes

  1. probably I thought I will iterate over the channel with for and it should be OK even if it was empty totally missing the behaviour of generateCommand function and the rm

  2. It probably doesn't work 😕

@jsproede
Copy link
Author

Hi @kucukaslan,

you're right. There aren't any changes :) Even the first sync (with --delete) tries to create a rm command, when the bucket is fully empty (as seen in the example).

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 a pull request may close this issue.

2 participants