Skip to content

Feat presign#634

Merged
igungor merged 6 commits intopeak:masterfrom
zemul:feat_presign
Aug 21, 2023
Merged

Feat presign#634
igungor merged 6 commits intopeak:masterfrom
zemul:feat_presign

Conversation

@zemul
Copy link
Contributor

@zemul zemul commented Aug 2, 2023

Used to share object urls.

@zemul zemul requested a review from a team as a code owner August 2, 2023 02:21
@zemul zemul requested review from igungor and seruman and removed request for a team August 2, 2023 02:21
Comment on lines +570 to +571
url, err := req.Presign(expire)
return url, err
Copy link
Member

Choose a reason for hiding this comment

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

You may return directly here: return req.Presign(expire).


func validatePresignCommand(c *cli.Context) error {
if c.Args().Len() != 1 {
return fmt.Errorf("expected only one argument")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return fmt.Errorf("expected only one argument")
return fmt.Errorf("expected a remote object url")

return resp.Body, nil
}

// Presign fetches the remote object url and returns its.
Copy link
Member

Choose a reason for hiding this comment

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

You may remove the comment altogether.

@igungor
Copy link
Member

igungor commented Aug 11, 2023

@zemul It'd be great to have a testcase for this command. You may take a look at e2e directory for command tests.

@igungor
Copy link
Member

igungor commented Aug 21, 2023

I'll go ahead and merge this PR without waiting for the OP because it's a useful feature and the more we wait, the more this PR will receive conflicts. I'll apply the changes myself later.

@igungor igungor merged commit 6a1ea3c into peak:master Aug 21, 2023
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.

3 participants