Skip to content

map prompt for duplicate creation #222

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

Closed
wants to merge 31 commits into from
Closed

map prompt for duplicate creation #222

wants to merge 31 commits into from

Conversation

mfordjody
Copy link
Contributor

@mfordjody mfordjody commented May 7, 2023

No description provided.

@@ -76,6 +77,13 @@ func NewCreateVolumeTask(curveadm *cli.CurveAdm, cc *configure.ClientConfig) (*t
subname := fmt.Sprintf("hostname=%s image=%s", hc.GetHostname(), cc.GetContainerImage())
t := task.NewTask("Create Volume", subname, hc.GetSSHConfig())

if len(cc.GetContainerImage()) == 0 {
Copy link
Collaborator

@Wine93 Wine93 May 9, 2023

Choose a reason for hiding this comment

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

Actually, there is a default value for client container image, so maybe it's an useless branch. AND:

  • you should add a step to do checking, maybe step.Lamba is a good choice.
  • what is the purpose of those changes.

if err != nil {
return t, nil
}
} else {
Copy link
Member

Choose a reason for hiding this comment

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

We should prompt based on the actual volume size and the expected, rather than the container image.

@opencurveadmin
Copy link

@Wine93 @tsonglew PTAL

@caoxianfei1
Copy link
Contributor

@mfordjody anything changed?

@mfordjody
Copy link
Contributor Author

mfordjody commented May 29, 2023

@mfordjody有什麼改變嗎?

I am currently unable to conduct testing because I have encountered the above situation @caoxianfei1

@caoxianfei1
Copy link
Contributor

caoxianfei1 commented Jun 5, 2023

@mfordjody有什麼改變嗎?

I am currently unable to conduct testing because I have encountered the above situation @caoxianfei1

you can delete the container and retry it.

@mfordjody mfordjody closed this by deleting the head repository Jul 29, 2023
@mfordjody
Copy link
Contributor Author

Requires longer modification time

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.

6 participants