search

Azure / azure-sdk

This is the Azure SDK parent repository and mostly contains documentation around guidelines and policies as well as the releases for the various languages supported by the Azure SDK.
http://azure.github.io/azure-sdk
MIT License
487 stars 297 forks source link

Board Review: Service Versions #1043

Closed heaths closed 4 years ago

heaths commented 4 years ago

Storage has already started supporting a new stable service version, while Key Vault is about to add support for a preview service version. We have many questions and would like to discuss and arrive at some guidance how Azure SDK should deal with service versions.

Scenario

What's driving this is that Key Vault is adding a 7.1-preview service version. This will likely be supported for some time before it GAs, and perhaps longer after that depending on service guidance. In response, we want to release a 4.1-preview client version that contains the new APIs they need to preview to customers.

Client version options

There are a couple of ways to do this, both with semvers or date versions:

  1. ClientOptions.ServiceVersion enumerations can define, for example, a V7_1_Preview version that equates to "7.1-preview". When the service GAs, we'd keep that version and add a V7_1 version when we release new packages. We could never drop V7_1_Preview even if the service eventually drops support.
  2. ClientOptions.ServiceVersion enumerations can define, for example, a V7_1 version that equates to "7.1-preview" and change that when the service GAs and we release new packages.

On a related note, does this necessitate the client being in a preview state in directly correlation to service being in a preview state? If it does, because the client is in a preview state can we drop the V7_1_Preview service version before we GA and after the service has GA'd?

Default client version

I can find no guidance on what the default service version should be. Key Vault currently defaults to "7.0" - our track 2 "RTM", if you will. It does define a constant LatestVersion value, however. Storage, on the other hand, defaults to the latest version.

  1. On one hand, defaulting to the latest helps push clients to the latest service version and eventually the service could deprecate older versions. Especially during previews, this can help uncover bugs in both the service and client.
  2. On the other hand, this could mean customers run into unexpected breaking service changes. As with typical libraries, devs don't like to update unless they have to. Is it their responsibility to pin to a particular version via ClientOptions, or should we do it for them through a fixed default or, perhaps, upgrade periodically and intentionally?

What should guidance be in this case?

Testing different service versions

The guidelines state that all versions should be tested and recorded. While ideal, we also currently have a considerable amount of space consumed by recorded tests. Should we follow the guidelines, or - for now - try to mitigate growing repo sizes and test a single version when possible? If so, should we test the default version (or higher, as needed for some methods or models) as decided above?

Inconsistencies

There are also inconsistencies throughout the languages, as pointed out by @jongio:

.NET

TypeScript/JavaScript

Python

Java

tg-msft commented 4 years ago

You can take a quick look at https://github.com/Azure/azure-sdk-for-net/pull/9582 to see how we're testing multiple versions of Storage.

tg-msft commented 4 years ago

The C# Guidelines weigh on the default version:

The version parameter must be required, and default to the latest supported service version.

But it doesn't draw a distinction between Previews and Stable. I'm not sure if the other languages are any more explicit.

I would imagine we'd ship

adrianhall commented 4 years ago

Scheduled for Mar-2

heaths commented 4 years ago

That distinction is important, and any guidelines on which version to use should be general. To be honest, I didn't even check the C# guidelines because I expected general guidelines for anything related to this. JS Key Vault, for example, is making a distinction between Preview and Stable.

But regarding service version specific, one lingering question is whether we should explicitly define constants/values (and in some languages, there is no distinction like JS) for "7.1" mapping to "7.1-preview" until it ships, or just "7.1-preview" and keep it forever even if the service doesn't. Or should we keep our packages preview until a service GAs then drop "-preview" and GA our packages? I imagine for many client packages they may want to preview some functionality while keeping the package GA, though I'm not sure if that's allowed either. That's one thing we need guidance on as well.

@jongio can you also distill the info you sent me offline and update this thread this week so people have time to grok it before our meeting on March 2nd?

heaths commented 4 years ago

I updated the original description with some information @jongio sent me offline.

adrianhall commented 4 years ago

Notes from architecture board:

Recording: https://msit.microsoftstream.com/video/1cb87c26-f4f7-4c98-b953-cea00c451b0d

ACTION: Adrian will submit a PR to add the following requirements to general guidelines

Recorded Tests:

ACTION:

Inconsistencies:

ACTION:

kyle-patterson commented 4 years ago

Closing out as the guideline changes have been completed.