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

daisy/: added variable parsing to resize-disk. #1280

Merged

Conversation

hegao12
Copy link
Contributor

@hegao12 hegao12 commented Jul 7, 2020

Currently, the resize-disk commmand can only take integer input and
cannot parse variables in a workflow. In order to allow variables in
this command, a new field "SizeGbVar string" is added, and then its
value is converted into int64 and stored in the original "SizeGb int64"
field. In this way, everything else except the ResizeDisk struct fields and
populate function remains untouched.

@google-oss-robot
Copy link
Collaborator

Hi @hegao12. Thanks for your PR.

I'm waiting for a GoogleCloudPlatform member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@zoran15
Copy link
Contributor

zoran15 commented Jul 7, 2020

/ok-to-test

@@ -30,13 +31,23 @@ type ResizeDisk struct {
compute.DisksResizeRequest
// Name of the disk to be resized
Name string
// SizeGb in string format to parse variables in workflow
SizeGbVar string "json:\"sizeGb,omitempty\""
Copy link
Contributor

Choose a reason for hiding this comment

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

You can call it SizeGb (the same as the int field in DiskResizeRequest). In populate, you can refer to the int field as DiskResizeRequest.SizeGb.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If so, all the others places that uses SizeGb will need to be changed to DiskResizeRequest.SizeGb. I'm not sure about all the places where it is used.

Copy link
Contributor

Choose a reason for hiding this comment

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

In our code base it's only step_resize_disks and associated tests. They already reference it as DiskResizeRequest.SizeGb.

For other uses, since Daisy now uses Go modules, it will still be possible to reference the old version of the code by third parties.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's renamed now. Commit is amended.
BTW, if this is merged, when should I expect a new release of Daisy?

Copy link
Contributor

Choose a reason for hiding this comment

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

This will be in the :latest build of Daisy within 10 minutes. Releases are manual so the earliest we can do it is tomorrow afternoon, once all the tests run.

Currently, the resize-disk commmand can only take integer input and
cannot parse variables in a workflow. In order to allow variables in
this command, a new field "SizeGbVar string" is added, and then its
value is converted into int64 and stored in the original "SizeGb int64"
field. In this way, everything else except the ResizeDisk struct fields and
populate function remains untouched.
@hegao12 hegao12 force-pushed the resize-disk-with-variable branch from 66c01cc to a8504f2 Compare July 7, 2020 20:14
@google-oss-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hegao12, zoran15

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@zoran15 zoran15 merged commit 0bf6234 into GoogleCloudPlatform:master Jul 8, 2020
@hopkiw hopkiw linked an issue Oct 2, 2020 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error in replacing variables in command resize-disk
3 participants