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

Add a client for Virtual Network management #33

Merged
merged 1 commit into from Feb 4, 2015

Conversation

jen20
Copy link
Contributor

@jen20 jen20 commented Feb 3, 2015

This pull requests support for managing virtual networks in Azure.

It adds structs to parse and generate the (seemingly unavailable) XML schema for virtual network specification, and a client with Get and Set operations for the currently active subscription.

The pattern for using the client is the same as the powershell version - get the running config, merge in any desired changes and set the configuration to the new state. There is no documentation on what the concurrency constraints are with the underlying Azure REST API, I suspect it is not safe to call concurrently though can't verify this.

It was also necessary to modify the HTTP client to accept different content types, since the underlying REST API requires the PUT operation to be made with a content type of text/plain, despite it actually being XML.

return err
}

fmt.Println("Content", string(networkConfigurationBytes))
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, that was an oversight from when I was trying to debug the schema. I'll remove.

@ahmetb
Copy link
Contributor

ahmetb commented Feb 3, 2015

Yeah API doesn't support concurrency. :) It's easy to run into race conditions in VNet REST APIs. Feel free to upvote my feedback: http://feedback.azure.com/forums/217313-networking-dns-traffic-manager-vpn-vnet/suggestions/5954992-make-virtual-network-operations-atomic-on-portal-a

//configuration for the currently active subscription. Note that the
//underlying Azure API means that network related operations are not safe
//for running concurrently.
func GetVirtualNetworkConfiguration() (*NetworkConfiguration, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't it be just NetworkConfiguration, without pointer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can see arguments for either here. Will look through what other clients do and potentially change to returning the value instead.

@jen20
Copy link
Contributor Author

jen20 commented Feb 3, 2015

@ahmetalpbalkan re: concurrency a reasonable way to do this would be to support If-Match on the tag of the configuration. I suspect that will not be supported by Azure any time soon though!

//currently active subscription according to the NetworkConfiguration given.
//Note that the underlying Azure API means that network related operations
//are not safe for running concurrently.
func SetVirtualNetworkConfiguration(networkConfiguration *NetworkConfiguration) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Check for null param here?

Copy link
Contributor

Choose a reason for hiding this comment

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

this should not be a pointer anyway.

@ruslangabitov
Copy link
Contributor

Can you fix Travis build please?

@ahmetb
Copy link
Contributor

ahmetb commented Feb 3, 2015

rebasing the commit on top of master should be fixing the problem.

@jen20
Copy link
Contributor Author

jen20 commented Feb 4, 2015

Yes, rebasing and changing the use of unexported symbols fixes this.

@jen20
Copy link
Contributor Author

jen20 commented Feb 4, 2015

By the way, what's the thought in this project of squashing commits prior to PR? Prefer the clean history or the complete change sets?

@ahmetb
Copy link
Contributor

ahmetb commented Feb 4, 2015

For the changes essentially changing one major thing, one squashed commit preferable. Thanks.

@jen20
Copy link
Contributor Author

jen20 commented Feb 4, 2015

Do you want me to squash this one?

@ahmetb
Copy link
Contributor

ahmetb commented Feb 4, 2015

Yes please.

This commit adds entities to generate the (unavailable) XML schema for
virtual network specification, and a client with Get and Set operations
for the currently active subscription.

The pattern for using the client is the same as the powershell version -
get the running config, merge in any desired changes and set the
configuration to the new state. There is no documentation on what the
concurrency constraints are with the underlying Azure REST API, I
suspect it is not safe to call concurrently though can't verify this.
@ahmetb
Copy link
Contributor

ahmetb commented Feb 4, 2015

LGTM

@jen20
Copy link
Contributor Author

jen20 commented Feb 4, 2015

👍 in use it's actually a bit derpy because of the semantics around modifying the "document" that gets returned when it's not a pointer. May revisit that in future.

ruslangabitov added a commit that referenced this pull request Feb 4, 2015
Add a client for Virtual Network management
@ruslangabitov ruslangabitov merged commit 0fbd371 into Azure:master Feb 4, 2015
@jen20 jen20 deleted the virtual-network-client branch February 5, 2015 16:40
marstr pushed a commit to marstr/azure-sdk-for-go that referenced this pull request Apr 27, 2017
* Including date and api version in request error

* Date as a string
marstr pushed a commit that referenced this pull request Apr 28, 2017
* Including date and api version in request error

* Date as a string
mcardosos added a commit that referenced this pull request May 4, 2017
* Including date and api version in request error

* Date as a string
richardpark-msft pushed a commit to richardpark-msft/azure-sdk-for-go that referenced this pull request Aug 5, 2021
make all examples and remove extra ctxs
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.

None yet

3 participants