Data Factory: repeated Breaking Changes to an API version from 2018

[tombuildsstuff]

👋

There’s currently two versions of the Data Factory API:

• 2017-09-01-preview - a Preview API that hasn’t received any notably changes in several years
• 2018-06-01 - a Stable API that regularly receives new features and breaking changes.

Now firstly, I enjoyed 2018 as much as anyone else, but it was also 6 years ago at this point.. frankly I find it surprising that a service that’s still in active development is still making changes, let alone breaking changes to a six year old API version on a regular basis.

Per the ARM Specification and Breaking Change processes - breaking changes are supposed to be introduced in a new API version, but perusing the Git history for this API version, regularly make it into this existing API Version, here’s a selection from the past year:

• 2 days ago, changing the type from string -> objects DataFactory: fix headers schema issue #28210
• 2 weeks ago: breaking change to the models (introducing a new new discriminated type ServicePrincipalCredentialResource - in addition to ManagedIdentityCredentialResource) - but crucially this doesn’t work since both models use the same Discriminator Value - Swagger changes for adding SPN, UAMI and SAMI credentials #28122
• 2 months ago: - breaking change to remove SalesforceV2SourceReadBehaviour Improve Salesforce V2 properties #27412
• 2 months ago: changing the type from object -> list Fix headers and schema definition #27180
• 3 months ago: the field pwd is removed: https://github.com/Azure/azure-rest-api-specs/pull/26965/files#diff-61ee9b50b0c4813e20b82ed9cd1cae9ce5fcabe739eec9aa61293957bc9b9c93L5312 - this could be because the field isn’t valid, but there’s no context in the PR so how can consumers know the reasoning for this?
• 4 months ago: breaking changes to the types fix swagger definition. #26846
• 6 months ago: Properties are removed and aren’t called out by the PR reviewers - in a PR containing 10 lines of changes - which was then fixed in this PR where it’s claimed these were “accidentally removed”.
• 9 months ago: breaking change to change mongoDbAtlasDriverVersion to driverVersion change MongoDbAtlasDriver name #24709
• 10 months ago: Constants are removed in favour of making the fields a string type that returns those same values Fix bug For Github Issues #24226
• 11 months ago: breaking change - string to object [Hub Generated] Publish private branch 'datafactory/binluwang/synctrack2' #23684
• 11 months ago: breaking change - boolean to object [Hub Generated] Publish private branch 'datafactory/liudan7/AdlsSwaggerSupportExpression' #23396

This happens so regularly that in order to populate this list, I haven’t even clicked onto the second page of Git history!

Each of these breaking changes has been approved, meaning that it must have gone through the breaking change process - yet in the PR description there’s no explanation of the reasoning for the change.

In addition to that the Data Factory API is documented using custom format values, for example:

"type": {
  "type": "string",
  "description": "The read setting type."
},
"maxConcurrentConnections": {
  "type": "object",
  "x-ms-format": "dfe-int",
  "description": "The maximum concurrent connection count for the source data store. Type: integer (or Expression with resultType integer)."
},
"disableMetricsCollection": {
  "type": "object",
  "x-ms-format": "dfe-bool",
  "description": "If true, disable data store metrics collection. Default is false. Type: boolean (or Expression with resultType boolean)."
}

As mentioned above, fields get accidentally removed which is missed at review time despite the PR containing 10 lines of changes - and then in a follow up PR to reintroduce these a complaint is raised about the type and discarded.

This PR updates a model so that it’s no longer marked as a Discriminated Type (and all implementations) and then later reintroduces them because it turns out people were using these types.

I can’t speak to each of these Swagger PRs because unfortunately they don’t contain the full context - but my point here is that for a Stable API version from 2018 there sure are a lot of changes, particularly Breaking Changes happening.

Questions

I understand the unique nature of this API is adding new Entity Types etc, so I understand adding (and potentially deprecating) Discriminator Values. However, at the same time, I can’t help but notice that the Data Factory API seems to regularly get breaking changes approved.

Whilst some older API versions do get a free pass with regards to fixing linting issues and similar, I suspect that because there hasn’t been another API version in so long that the Data Factory API is intentionally running below the radar here.

As such:

1. Can the ARM Team confirm what’s happening here? Why there’s been so many breaking changes to this API version?
2. Can the Service Team confirm when a new API version will be introduced? As much as I appreciate reminiscing about 2018, it was 6 years ago at this point, so it feels like there should be a 2024 version ;)
3. Why is this Service using custom format values? There’s a perfectly good type field in the Swagger/OpenAPI specification for this purpose, and outputting booleans as object is.. unique.
4. Are there plans to fix this Swagger definition? In particular most of the Discriminated Types don’t define a x-ms-discriminator-value value.

Thanks!

[lirenhe]

cc @mikekistler and @JeffreyRichter for awareness.

[konrad-jamrozik]

@tombuildsstuff re this:

Each of these breaking changes has been approved, meaning that it must have gone through the breaking change process - yet in the PR description there’s no explanation of the reasoning for the change.

We recently switched to a model where the approval labels provide general reason in them why given approval happened:

• [Breaking Change][PR Workflow] Use more granular labels for Breaking Changes approvals azure-sdk-tools#6374

[tombuildsstuff]

Another breaking change: #28538

Note the PR Description:

[konrad-jamrozik]

@mikekistler @JeffreyRichter

[JeffreyRichter]

Mike & I are talking to the Data Factory team about all these breaks to see how we can improve the customer experience.
@tombuildsstuff: Please keep reporting these and it gives the team evidence of the problems and customer pain to raise the importance of fixing their versioning strategy.

[tombuildsstuff]

Another breaking change here: #28294 - with no details as to the need for this breaking change in the PR description/thread (despite being asked to provide that in a comment):

But apparently this has been approved by the ARM Breaking Changes team?

This is exposed as a hard field rename:

Which shows for us here:

https://github.com/hashicorp/pandora/pull/4138/files#diff-a46695a73172fc26f0a3fb681b1298f7c4b002386c5f80aa4459fd6d943d568dR23