Skip to content
This repository was archived by the owner on Mar 27, 2023. It is now read-only.

Conversation

@yi0909
Copy link
Contributor

@yi0909 yi0909 commented Oct 24, 2022

This change includes

  • Introduce VirtualMachinePublishRequest reason.
  • Update MarkCondition function. It should set the reason to the input parameter if it is not empty.

This change includes
- Introduces a field Attempt to the VirtualMachinePublishRequestStatus.
Attempt represents the number of times VM publish request has been attempted.
- Introduce a field LastAttemptTime to the status, which represents the time when the last attempt was sent.
- Update VirtualMachinePublishRequestTarget comment.
@yi0909 yi0909 force-pushed the topic/yijiang/add-vmpub-condition-reason branch from 23afcba to d3ae311 Compare October 25, 2022 23:29
This change includes
- Introduce VirtualMachinePublishRequest reason.
- Update MarkCondition function. It should set the reason to the input parameter if it is not empty.
@yi0909 yi0909 force-pushed the topic/yijiang/add-vmpub-condition-reason branch from d3ae311 to 1596fe7 Compare October 26, 2022 19:00
Copy link
Contributor

@zyiyi11 zyiyi11 left a comment

Choose a reason for hiding this comment

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

Overall LGTM. Just VirtualMachinePublishRequestUploadingReason = "Uploading" seems odd, it's more like a phase.


// VirtualMachinePublishRequestSourceVMNotCreatedReason (Severity=Error) documents that the source VM
// hasn't been fully created yet.
VirtualMachinePublishRequestSourceVMNotCreatedReason = "SourceVMNotCreated"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have sufficient context to determine between this reason and the UniqueIDNotReady below? Once the VM is created, we have the MoID/UniqueID. What does "fully created" mean in the context here?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants