Fix/6584 content type for lakectl local #6644
Conversation
There was a problem hiding this comment.
Hey @0x-chaitu - thank you for your efforts. We value every contribution to this project, it's all about the community :)
Regarding the code, notice that unlike lakectl fs upload ..., lakectl local commit ... may upload many objects with different content types at once. When the user request for it explicitly using a flag, we should identify the content automatically and not rely on a single given content that cannot fit all types. Other than that, the PR looks very promising.
itaiad200
added
contributor
include-changelog
|
Thanks @itaiad200, |
| localCommitAllowEmptyMessage = "allow-empty-message" | ||
| localCommitMessageFlagName = "message" | ||
| localContentTypeFlagName = "content-type" | ||
| localIncludeContentTypeFlagName = "include-content-type" |
There was a problem hiding this comment.
| localIncludeContentTypeFlagName = "include-content-type" | |
| localIncludeContentTypeFlagName = "detect-content-type" |
| @@ -120,7 +124,7 @@ var localCommitCmd = &cobra.Command{ | |||
| }() | |||
| sigCtx := localHandleSyncInterrupt(cmd.Context(), idx, string(commitOperation)) | |||
| s := local.NewSyncManager(sigCtx, client, syncFlags.parallelism, syncFlags.presign) | |||
| err = s.Sync(idx.LocalPath(), remote, c) | |||
| err = s.Sync(idx.LocalPath(), remote, c, contentType, includeContentType) | |||
There was a problem hiding this comment.
I don't understand. When would I want to set contentType to the same value on all the files in a Sync?
THANKSfor picking up this issue and working to fix it. This is really appreciated, and a great way to improve lakeFS as a product. |
|
Hi @0x-chaitu, could you please provide an update on the PR status? |
Hi @nopcoder, @itaiad200 has self-assigned the issue. |
|
Hey @0x-chaitu , I self-assigned the PR so I could track this better. We do that normally for all contributors. Do you still intend to work on this PR or should we take it? |
|
Hey, @itaiad200 Please take it. |
|
This PR is now marked as stale after 30 days of inactivity, and will be closed soon. To keep it, mark it with the "no stale" label. |
|
Closing this PR because it has been stale for 7 days with no activity. |
Closes #6584
Change Description
Background
Followed code from lakectl fs upload.
Bug Fix
If this PR is a bug fix, please let us know about:
Testing Details
How were the changes tested?
They were tested by running lakectl local to upload files with content-type flag
Contact Details
[email protected]