Skip to content

fix: cp command S3toS3 --content-type flag #739

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Jul 26, 2024
Merged

fix: cp command S3toS3 --content-type flag #739

merged 11 commits into from
Jul 26, 2024

Conversation

4o4x
Copy link
Contributor

@4o4x 4o4x commented Jul 10, 2024 •
edited
Loading

In s5cmd, the --content-type flag does not work when performing an S3 to S3 copy operation with the cp command.

Added tests addressing this issue.

resolves #738

@4o4x 4o4x requested a review from a team as a code owner July 10, 2024 15:29
@4o4x 4o4x requested review from igungor and denizsurmeli and removed request for a team July 10, 2024 15:29
@denizsurmeli
Copy link
Contributor

Hi, thanks for the PR, can you edit the PR description that indicating #738 is resolved by adding "Resolves #PR" in a seperate line. You can take a look at this for full reference.

@ilkinulas
Copy link
Member

@4o4x is this PR ready for review? if not ready, convert it to draft please.

@4o4x 4o4x marked this pull request as draft July 12, 2024 10:03
@4o4x 4o4x marked this pull request as ready for review July 12, 2024 12:59
@denizsurmeli
Copy link
Contributor

@4o4x tests fail. Can you fix them? Then the PR can be reviewed.

denizsurmeli
denizsurmeli requested changes Jul 12, 2024
View reviewed changes
denizsurmeli
denizsurmeli reviewed Jul 12, 2024
View reviewed changes
denizsurmeli
denizsurmeli reviewed Jul 23, 2024
View reviewed changes
denizsurmeli
denizsurmeli reviewed Jul 23, 2024
View reviewed changes
denizsurmeli
denizsurmeli reviewed Jul 23, 2024
View reviewed changes
denizsurmeli
denizsurmeli reviewed Jul 23, 2024
View reviewed changes
denizsurmeli
denizsurmeli previously approved these changes Jul 23, 2024
View reviewed changes
igungor
igungor requested changes Jul 26, 2024
View reviewed changes
Copy link
Member

@igungor igungor left a comment

Choose a reason for hiding this comment

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

Could you update Changelog file?

4o4x and others added 10 commits July 26, 2024 16:07
@4o4x
test: Added new test with --content-type test
04a66b5
@4o4x
chore: Refactor variable names in cp_test.go for clarity
7eeca89
@4o4x
fix: fixed content type change in cp command
672f93f
@4o4x
fix: Skip file copy test on Windows due to invalid characters
ef26b5b 
The test `TestCopyMultipleFilesToS3BucketWithContentType` is now skipped on Windows due to the presence of invalid characters in file names. This prevents the test from failing on Windows machines.
@4o4x
chore: revert unnecessarily changes
abb38a4
@4o4x
refactor refactor some copy tests
4043a3a
@4o4x
fix: Fix meta-directive default value in copy command when s3tos3
bd94303
@4o4x
style: changed a variable name
fcface8
@4o4x @denizsurmeli
refactor: windows skip moved to bottom of t.Parallel()
7e0959d 
Co-authored-by: Deniz Surmeli <52484739+denizsurmeli@users.noreply.github.com>
@4o4x
Update CHANGELOG.md
bafc979
@4o4x 4o4x force-pushed the issiue738-cp-content-type branch from b3fd16f to bafc979 Compare July 26, 2024 13:08
igungor
igungor reviewed Jul 26, 2024
View reviewed changes
@igungor igungor merged commit c280956 into peak:master Jul 26, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@igungor igungor igungor approved these changes

@denizsurmeli denizsurmeli denizsurmeli approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

cp command S3toS3 --content-type flag not working
4 participants
@4o4x @denizsurmeli @ilkinulas @igungor