Skip to content

VM Connect Changes#309

Open
hvedati wants to merge 9 commits intomicrosoft:masterfrom
hvedati:dev/hv/vmconnect
Open

VM Connect Changes#309
hvedati wants to merge 9 commits intomicrosoft:masterfrom
hvedati:dev/hv/vmconnect

Conversation

@hvedati
Copy link
Contributor

@hvedati hvedati commented Mar 17, 2025

No description provided.

@hvedati hvedati changed the title Dev/hv/vmconnect VM Connect Changes Mar 18, 2025
@hvedati
Copy link
Contributor Author

hvedati commented Apr 11, 2025

/azp run

@hvedati hvedati marked this pull request as ready for review April 11, 2025 20:48
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@hvedati
Copy link
Contributor Author

hvedati commented Apr 11, 2025

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@hvedati
Copy link
Contributor Author

hvedati commented Apr 11, 2025

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@hvedati
Copy link
Contributor Author

hvedati commented Apr 11, 2025

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@@ -94,7 +94,7 @@ func GetServerAddress(cc *grpc.ClientConn) string {
defer mux.Unlock()

Choose a reason for hiding this comment

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

probably can just remove this file to keep the commit history simple

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in new pr.

@@ -39,19 +39,21 @@ require (
github.com/spf13/pflag v1.0.5 // indirect

Choose a reason for hiding this comment

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

same as other prs, make sure to remove all changes from these files before merge

return nil, err
}

mocResponse, err := c.VirtualMachineAgentClient.GetHyperVVmId(ctx, vm[0])

Choose a reason for hiding this comment

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

Just to be safe, can we make sure the vm list is non-empty before accessing it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in new pr.

return nil, err
}

mocResponse, err := c.VirtualMachineAgentClient.GetHyperVVmId(ctx, vm[0])

Choose a reason for hiding this comment

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

Is there a specific reason we aren’t using the INVOKE function for the calls to cloudAgent? It already has RBAC, redaction, and common logging built in.

Would it make sense to embed these new function calls within INVOKE to take advantage of those built-in features?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These functions are not running operations on the VM, they are simply getting and passing up information from set fields.

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.

4 participants