V9 #569

marstr · 2017-03-09T19:32:54Z

For Version 9.0.0-beta, there are a bunch of breaking changes that we've been holding off on until we could give folks more of a proper heads-up.

So, the goal of this PR is to capture a running list of categories of changes (breaking and otherwise) that will be present in v9 and the packages impacted by them. CHANGELOG will contain a summary of the changes as well, but could never hope to be as precise as the diffs in a PR.

I'll be keeping the v9 branch in my fork until this is merged in. So if you have questions about whether or not a package you're interested in will be impacted, I recommend cloning my fork then doing a diff of a specific package like so:

go get -u github.com/marstr/azure-sdk-for-go
cd $GOPATH/src/github.com/marstr/azure-sdk-for-go
git checkout v9
git diff v8.1.1-beta v9 -- arm/<package name>

Note, there will be more revisions to this PR through-out March until we release. The current status is intended to demonstrate name changes, etc in packages where the Swaggers were not updated.

* Updated Gen.go to use new autorest builld system * Comments from review * It works as expected again

* Updating CI to use Go 1.8. Go 1.8 was released on Thursday, February 16th. Many of our partners have adopted and we need to ensure that we support the most recent version. * Replacing storage folder with submodule. In order to allow for more independent growth of the storage package, we've given it its own repository. This change updates the main repository to use a submodule in order to prevent breaking changes for folks using storage today who do not want to migrate to the new repository. * Updating submodule target URL. There is an existing naming convention for Azure Storage repositories. the repo has been updated to match that convention, this updates the submodule to point to the new URL. * Responding to review feedback. * Updating submodule to ref v8.1.0 storage version.

marstr · 2017-03-09T19:36:22Z

First Major Change to note:
Storage has been moved into its own repository. We've added a submodule so that this isn't a breaking change, but consuming it through Azure/azure-sdk-for-go is deprecated. We encourage you to directly consume it via Azure/azure-storage-go

marstr · 2017-03-09T19:48:18Z

There are breaking name changes in the following packages:

arm/authorization
arm/batch
arm/dns
arm/eventhub
arm/logic
arm/notificationshub
arm/redis
arm/sql
datalake-analytics/account
datalake-store/filesystem

The Location field has erroneously been removed from the Service Bus Response models in v9.0.0-beta. To support folks without forcing further breaking changes, we'll release a v9.1.0-beta which.

(I'll edit this list to add the full list of impacted packages as I discover them. Please check back here before you adopt v9.0.0-beta)

edit : March 31, 2017 11:12 AM PDT
edit2: April 20th, 2017 4:05 PM PDT

marstr · 2017-03-09T19:50:05Z

#559 is fixed in this PR. Version number and UserAgent are now handled by the generator instead of being created at runtime. This may have to change if we want to start capturing Go Version or OS as part of the UserAgent field.

randomcamel · 2017-03-09T20:49:13Z

Is there a way to manage compatibility issues among the newly-separated libraries? How do we know what versions are safe to use together?
This is a pretty heavy change: is there a doc somewhere with the backstory on what problems it's solving and how this solution emerged?

marstr · 2017-03-09T23:37:56Z

Howdy @randomcamel,

Very valid concern! For some context around why we're pulling storage into a new repository, you can read #517. At the end of the day, what it really comes down to is protecting our storage/ASM users from the frequent massive PRs and Major Version bumps that come along with the auto-generated ARM packages.

In terms of compatibility, our decision was made easier by the fact that they are completely disparate packages. Other than spinning up new storage accounts, there isn't really any overlap between the storage and ARM packages. All versions are compatible with one another. We'll eventually be making a similar change to the "management" (ASM) packages.

As to the other changes in this PR, they are the result of an aggregate monthly publish from swagger files in the Azure/azure-rest-api-specs repository. Getting as many customers away from these bloated PRs as possible is important to us. Long term, we're working on a continuous deployment story for the ARM packages.

I hope that answers your questions! Feel free to follow up or ask more.

-Martin.

pmcatominey · 2017-03-14T16:13:21Z

While the storage and arm packages are wholly separate, they do both depend on the Azure/go-autorest package. The only concern there would be if a breaking change was made to autorest and only one of arm or storage had been updated.

tombuildsstuff

👋 hey @marstr

I've imported these changes into the Terraform codebase to take a look at our upgrade path. Regarding the ARM SDK changes - the only concern I've got is regarding the missing fields - presumably these have been Deprecated?

A couple of questions:

Is there any kind of Deprecation policy available for fields in the SDK? e.g. a setting in the Swagger? If so - would it be possible to add this to the SDK release notes?
How strict are the contracts on API versions? Presumably fields would be removed (or data types changed) in a subsequent version, or is there potential to change within a single version?

Regarding the fields - are they likely to make a return before V9 lands?

Thanks!

tombuildsstuff · 2017-03-16T17:28:27Z

arm/servicebus/models.go

-	EnableExpress                       *bool                    `json:"enableExpress,omitempty"`
-	EnablePartitioning                  *bool                    `json:"enablePartitioning,omitempty"`
-	EnableSubscriptionPartitioning      *bool                    `json:"enableSubscriptionPartitioning,omitempty"`
-	FilteringMessagesBeforePublishing   *bool                    `json:"filteringMessagesBeforePublishing,omitempty"`


We're currently exposing this field, but were unaware it was Deprecated / scheduled to be removed.

I can see it's been removed from the Swagger in this commit - however presumably it's still being returned in the API for the moment, is that likely to change?

Good news on this front, @tombuildsstuff. We've reached out to the service team, and they've agreed to publish these changes as part of a new API Version instead of modifying the existing one. (Which is more consistent with the way API Versions are supposed to work.) That means that this change will not be part of V9.

@marstr that's great - thanks! :)

tombuildsstuff · 2017-03-16T17:31:19Z

arm/compute/models.go

@@ -397,7 +374,6 @@ type AvailabilitySet struct {
 	Location                   *string             `json:"location,omitempty"`
 	Tags                       *map[string]*string `json:"tags,omitempty"`
 	*AvailabilitySetProperties `json:"properties,omitempty"`
-	Sku                        *Sku `json:"sku,omitempty"`


we're currently reliant on this field to expose whether an Availability Set is Managed or not - has it been Deprecated?

(OT: what does it mean for an AS to be managed or not, any docs on that?)

Sorry - missed this! Digging our use - we're setting the SKU to aligned which according to the initial PR means the Availability Set can contain VM's with Managed Disks?

@tombuildsstuff, looks like this was a mistake on my part! I generated compute from the latest non-preview API Version. It looks like we've been generating from the version 2016-04-30-preview in the past. I've updated our manifest and am regenerating now. For v10, we'll generate from the composite swagger and this won't be an issue.

Using Azure/azure-rest-api-specs commit: 7d9b6c0

marstr · 2017-03-17T17:43:21Z

Howdy @tombuildsstuff ,

@kirthik and I are digging into these particular changes for you. We don't have all of the information yet, but from our initial conversations, it seems like these fields are not going to comeback for v9.0.0-beta.

For the broader conversation around deprecation, I think you have the right read on things. The services for a given API Version are supposed to only change for bugs, the contracts should never change. Between API Versions, all bets are off, the entire world can change. Because former API Versions will still be supported by the Services, tying yourself to an API Version for an operation is the way to increase stability.

At the moment, that is pretty awkward for the Azure-SDK-for-Go. We only generate the SDK from the most recent API Version, then support for former versions is left to just over-writing the API Version field (which isn't really an answer to the problem at all.) #517 brought up an interesting point, which is that we really need to support all API Versions that the Services themselves support. Our team is currently floating around a few ideas for how we're going to do that, and are running some experiments right now. Particularly, the semantics of composite swaggers don't play super well with how we were thinking of packaging multiple API Versions. I'll open up an issue to track that conversation here very soon.

tl;dr In theory, deprecation should apply to whole API Versions, not individual fields. API Versions should each have long-term support. We're working on updating the Azure-SDK-for-Go to expose that awesome theoretical stability to our customers.

edit: finishing up a dangling sentence

The new API Version strategy associated the API Version to send across the wire with each method instead of at the client level. Doing so allows us to support a broader range of composite swaggers (the immediate example is the web composite swagger.) It also means that making a call with an alternate API version is somewhat more difficult in this version. To do so, one would have to create their own Preparer for the request. In the future, we'll support multiple API Versions and allow for more stability by generating independent packages for each Operation Group in each API Version. We expect to have that done in the summer.

marstr · 2017-03-29T17:22:25Z

Hey @tombuildsstuff, mind taking another look to ensure that the most recent version has fixed the Service Bus issues that you were seeing?

tombuildsstuff · 2017-03-29T17:33:40Z

@marstr taking a quick look at the GitHub diff it LGTM - I'll update my fork and do another merge tomorrow :)

tombuildsstuff · 2017-03-30T18:25:20Z

👋 hey @marstr

I've taken a look into this today (excluding the Storage API changes, which are being handled separately) - and there's one major blocker from migrating to v9 at our end - which is the removal of the SKU property from the Availability Set. From what I can see, we'd need this functionality in order to be able to ship the Managed Disks PR, which is pretty highly requested.

Is there any chance this will be reverted by the time that the v9 SDK is released? (I was hoping to link to it, but I'm struggling to find the Swagger PR associated with that change)

FWIW one subtle thing I've noticed whilst working through this is that fields in the Swagger can have spaces in the names, i.e. the API field duplicateDetectionHistoryTimeWindow was in the Swagger as duplicateDetectionHistoryTimeWindow (note the space). If there isn't already, is there anything AutoRest could do to handle/catch these kind of issues in future?

FYI I've pushed the branch with the changes here - and we're going to run the full test suite overnight :)

Thanks! :)

tombuildsstuff · 2017-03-31T14:54:05Z

Hey @marstr

Just to follow up: I've run the AzureRM test suite a couple of times and aside from the issues I've pointed out above - it looks good :)

Thanks!

marstr · 2017-03-31T15:48:29Z

Thanks @tombuildsstuff! I'll follow up by EOD with status of what I've learned about that field.

edit: Done, see sub-thread above.

tombuildsstuff · 2017-03-31T16:57:57Z

@marstr thanks a ton for all the extra effort here btw :)

marstr · 2017-03-31T16:59:01Z

Not a problem, @tombuildsstuff. :)

marstr and others added 3 commits March 9, 2017 10:25

Running latest Go Generator on all released packages.

eb9ce00

Gen.go update (Azure#547)

adb839f

* Updated Gen.go to use new autorest builld system * Comments from review * It works as expected again

marstr added this to the v9.0.0-beta milestone Mar 9, 2017

marstr self-assigned this Mar 9, 2017

msftclas added the cla-not-required label Mar 9, 2017

Fixing issues found while reviewing PR.

0f78b9a

marstr mentioned this pull request Mar 14, 2017

Documenting plans for 9.0 #529

Closed

tombuildsstuff reviewed Mar 16, 2017

View reviewed changes

Regenerating using latest swaggers.

b64cb91

Using Azure/azure-rest-api-specs commit: 7d9b6c0

marstr mentioned this pull request Mar 28, 2017

Removing APIVersion from client. Azure/autorest#2080

Merged

marstr added 2 commits March 28, 2017 16:36

Regenerating against most recent swaggers.

452b446

Regenerating using latest preview version of compute.

b25cadf

marstr merged commit eb9eddf into Azure:master Mar 31, 2017

marstr deleted the v9 branch March 31, 2017 17:10

vikstrous mentioned this pull request Apr 3, 2017

submodule file conflicts break glide/git #581

Closed

tombuildsstuff mentioned this pull request Apr 19, 2017

Regenerate the Service Bus SDK #593

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

V9 #569

V9 #569

marstr commented Mar 9, 2017

marstr commented Mar 9, 2017

marstr commented Mar 9, 2017 •

edited

Loading

marstr commented Mar 9, 2017

randomcamel commented Mar 9, 2017

marstr commented Mar 9, 2017

pmcatominey commented Mar 14, 2017

tombuildsstuff left a comment

tombuildsstuff Mar 16, 2017

marstr Mar 22, 2017 •

edited

Loading

tombuildsstuff Mar 23, 2017

tombuildsstuff Mar 16, 2017

colemickens Mar 16, 2017

tombuildsstuff Mar 30, 2017

marstr Mar 31, 2017

marstr commented Mar 17, 2017 •

edited

Loading

marstr commented Mar 29, 2017

tombuildsstuff commented Mar 29, 2017

tombuildsstuff commented Mar 30, 2017

tombuildsstuff commented Mar 31, 2017

marstr commented Mar 31, 2017 •

edited

Loading

tombuildsstuff commented Mar 31, 2017

marstr commented Mar 31, 2017

V9 #569

V9 #569

Conversation

marstr commented Mar 9, 2017

marstr commented Mar 9, 2017

marstr commented Mar 9, 2017 • edited Loading

marstr commented Mar 9, 2017

randomcamel commented Mar 9, 2017

marstr commented Mar 9, 2017

pmcatominey commented Mar 14, 2017

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff Mar 16, 2017

Choose a reason for hiding this comment

marstr Mar 22, 2017 • edited Loading

Choose a reason for hiding this comment

tombuildsstuff Mar 23, 2017

Choose a reason for hiding this comment

tombuildsstuff Mar 16, 2017

Choose a reason for hiding this comment

colemickens Mar 16, 2017

Choose a reason for hiding this comment

tombuildsstuff Mar 30, 2017

Choose a reason for hiding this comment

marstr Mar 31, 2017

Choose a reason for hiding this comment

marstr commented Mar 17, 2017 • edited Loading

marstr commented Mar 29, 2017

tombuildsstuff commented Mar 29, 2017

tombuildsstuff commented Mar 30, 2017

tombuildsstuff commented Mar 31, 2017

marstr commented Mar 31, 2017 • edited Loading

tombuildsstuff commented Mar 31, 2017

marstr commented Mar 31, 2017

marstr commented Mar 9, 2017 •

edited

Loading

marstr Mar 22, 2017 •

edited

Loading

marstr commented Mar 17, 2017 •

edited

Loading

marstr commented Mar 31, 2017 •

edited

Loading