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: allow mirror bucket/object/group using name #250

Merged
merged 2 commits into from
May 22, 2023

Conversation

forcodedancing
Copy link
Contributor

Description

This pr will allowing mirroring bucket/object/group by using id or name.

Rationale

By using name, we can package some messages in a single tx, e.g., CreateBucket MirrorBucket in one tx.

Example

NA

Changes

Notable changes:

  • cli changes
  • ...

@@ -756,11 +756,13 @@ $ %s tx delete-policy 3

func CmdMirrorBucket() *cobra.Command {
cmd := &cobra.Command{
Use: "mirror-bucket [bucket-id]",
Use: "mirror-bucket [bucket-id] [bucket-name]",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it is a bad parameter setting, for example if I provide bucket-id, I dont need to provide bucket-name. I can not set the bucket name only for the current implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I changed it to use flags for both bucket-id and bucket-name.

if msg.Id.GT(sdk.NewUint(0)) {
objectInfo, found = k.Keeper.GetObjectInfoById(ctx, msg.Id)
} else {
objectInfo, found = k.Keeper.GetObjectInfo(ctx, msg.BucketName, msg.ObjectName)
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a bit confused. Any checks will added to make sure the ID and name correspond to each other? Or if the name is specified, the id must be zero, Or if the ID is greater than 0, the name must be empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. The check had been added to the ValidateBasic functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants