BatchAI. Several fixes #3038

AlexanderYukhanov · 2018-05-08T16:27:42Z

This checklist is used to make sure that common issues in a pull request are addressed. This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.

PR information

The title of the PR is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
If applicable, the PR references the bug/issue that it fixes.
Swagger files are correctly named (e.g. the api-version in the path should match the api-version in the spec).

Quality of Swagger

I have read the contribution guidelines.
My spec meets the review criteria:
- The spec conforms to the Swagger 2.0 specification.
- The spec follows the guidelines described in the Swagger checklist document.
- Validation tools were run on swagger spec(s) and have all been fixed in this PR.

It fixes auto-generated clients.

Unique subdirectory will be only generated for jobs because only jobs generate output and so may have conflict with previous job versions.

AutorestCI · 2018-05-08T16:28:47Z

Automation for azure-sdk-for-python

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-python#2484

azuresdkciprbot · 2018-05-08T16:35:18Z

AutoRest linter results for ARM Related Validation Errors/Warnings

These errors are reported by the ARM team's validation tools, reachout to ARM RP API Review directly for any questions or concerns.

File: specification/batchai/resource-manager/readme.md

⚠️

0 new Warnings.(0 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-05-08T16:35:18Z

AutoRest linter results for SDK Related Validation Errors/Warnings

These errors are reported by the SDK team's validation tools, reachout to ADX Swagger Reviewers directly for any questions or concerns.

File: specification/batchai/resource-manager/readme.md

⚠️

0 new Warnings.(5 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

sergey-shandar · 2018-05-09T18:19:43Z

specification/batchai/resource-manager/Microsoft.BatchAI/stable/2018-05-01/BatchAI.json

@@ -3774,8 +3774,18 @@
      },
      "description": "Values returned by the List operation."
    },
+    "ExperimentBaseProperties": {
+      "properties": {


What's the value to have the empty type?

If we do not specify it, autogenerated C# client still has ExperimentCreationParameter which has type Object. Object will be very confusing for users.

OK, I dont understand this change and looks like it is added for the purpose of better experience in SDK.

@AlexanderYukhanov I mean, what's the purpose of the type?

Ok, I'm missing something here.

ExperimentBaseProperties doesn't contain anything

ExperimentCretationParameters only has a reference to ExperimentBaseProperties and declares x-ms-flatten .. What exactly are you asking users to pass into this function? What good are empty objects without any definition?

We just need to have an experiment create api which doesn't require any parameters but without this helper structure the generated code looks like this:

public async Task<AzureOperationResponse<Experiment>> CreateWithHttpMessagesAsync(string resourceGroupName, string workspaceName, string experimentName, object parameters, Dictionary<string, List<string>> customHeaders = null, CancellationToken cancellationToken = default(CancellationToken))

Please notice "oject parameters" in the signature.
If we add this structure the parameters have reasonable type ExperimentCreateParameters and become optional.

We have removed ExperimentCreationParameters completely now.

ravbhatnagar

ARM does not need to sign off on this change. Anyway, this is something done for better generated code. removing the ARM signoff label.

AutorestCI · 2018-05-15T23:42:01Z

Automation for azure-sdk-for-node

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-node#2809

AutorestCI · 2018-05-15T23:43:00Z

Automation for azure-libraries-for-java

The initial PR has been merged into your service PR:
AutorestCI/azure-libraries-for-java#89

AutorestCI · 2018-05-15T23:44:29Z

Automation for azure-sdk-for-go

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-go#1841

azuresdkciprbot · 2018-05-15T23:44:57Z

AutoRest linter results for ARM Related Validation Errors/Warnings

These errors are reported by the ARM team's validation tools, reachout to ARM RP API Review directly for any questions or concerns.

File: specification/batchai/resource-manager/readme.md

⚠️

0 new Warnings.(0 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-05-15T23:44:57Z

AutoRest linter results for SDK Related Validation Errors/Warnings

These errors are reported by the SDK team's validation tools, reachout to ADX Swagger Reviewers directly for any questions or concerns.

File: specification/batchai/resource-manager/readme.md

⚠️

0 new Warnings.(4 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

AlexanderYukhanov added 2 commits May 8, 2018 09:24

BatchAI. Added ExperimentBaseProperties

b7570a5

It fixes auto-generated clients.

BatchAI. Updated examples

8f361f3

Unique subdirectory will be only generated for jobs because only jobs generate output and so may have conflict with previous job versions.

sarangan12 added the review label May 8, 2018

azuresdkci assigned sergey-shandar May 8, 2018

azuresdkci requested a review from sergey-shandar May 8, 2018 16:28

AutorestCI mentioned this pull request May 8, 2018

[AutoPR batchai/resource-manager] BatchAI. Several fixes Azure/azure-sdk-for-python#2548

Merged

sergey-shandar added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label May 9, 2018

sergey-shandar reviewed May 9, 2018

View reviewed changes

ravbhatnagar reviewed May 15, 2018

View reviewed changes

ravbhatnagar removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label May 15, 2018

BatchAI. Remove CreateExperimentParameters

df89a61

AutorestCI mentioned this pull request May 15, 2018

[AutoPR batchai/resource-manager] BatchAI. Several fixes Azure/azure-sdk-for-node#2858

Merged

This was referenced May 15, 2018

[AutoPR batchai/resource-manager] BatchAI. Several fixes AutorestCI/azure-libraries-for-java#160

Merged

[AutoPR batchai/resource-manager] BatchAI. Several fixes Azure/azure-sdk-for-go#1836

Merged

sergey-shandar added the potential-sdk-breaking-change label May 16, 2018

sergey-shandar approved these changes May 16, 2018

View reviewed changes

sergey-shandar merged commit 7b1a493 into Azure:master May 16, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BatchAI. Several fixes #3038

BatchAI. Several fixes #3038

AlexanderYukhanov commented May 8, 2018 •

edited by azuresdkci

Loading

AutorestCI commented May 8, 2018 •

edited

Loading

azuresdkciprbot commented May 8, 2018

azuresdkciprbot commented May 8, 2018

sergey-shandar May 9, 2018

AlexanderYukhanov May 9, 2018

ravbhatnagar May 15, 2018

sergey-shandar May 15, 2018

fearthecowboy May 15, 2018

AlexanderYukhanov May 15, 2018

AlexanderYukhanov May 15, 2018

ravbhatnagar left a comment

AutorestCI commented May 15, 2018 •

edited

Loading

AutorestCI commented May 15, 2018 •

edited

Loading

AutorestCI commented May 15, 2018 •

edited

Loading

azuresdkciprbot commented May 15, 2018

azuresdkciprbot commented May 15, 2018

BatchAI. Several fixes #3038

BatchAI. Several fixes #3038

Conversation

AlexanderYukhanov commented May 8, 2018 • edited by azuresdkci Loading

PR information

Quality of Swagger

AutorestCI commented May 8, 2018 • edited Loading

Automation for azure-sdk-for-python

azuresdkciprbot commented May 8, 2018

AutoRest linter results for ARM Related Validation Errors/Warnings

azuresdkciprbot commented May 8, 2018

AutoRest linter results for SDK Related Validation Errors/Warnings

sergey-shandar May 9, 2018

Choose a reason for hiding this comment

AlexanderYukhanov May 9, 2018

Choose a reason for hiding this comment

ravbhatnagar May 15, 2018

Choose a reason for hiding this comment

sergey-shandar May 15, 2018

Choose a reason for hiding this comment

fearthecowboy May 15, 2018

Choose a reason for hiding this comment

AlexanderYukhanov May 15, 2018

Choose a reason for hiding this comment

AlexanderYukhanov May 15, 2018

Choose a reason for hiding this comment

ravbhatnagar left a comment

Choose a reason for hiding this comment

AutorestCI commented May 15, 2018 • edited Loading

Automation for azure-sdk-for-node

AutorestCI commented May 15, 2018 • edited Loading

Automation for azure-libraries-for-java

AutorestCI commented May 15, 2018 • edited Loading

Automation for azure-sdk-for-go

azuresdkciprbot commented May 15, 2018

AutoRest linter results for ARM Related Validation Errors/Warnings

azuresdkciprbot commented May 15, 2018

AutoRest linter results for SDK Related Validation Errors/Warnings

AlexanderYukhanov commented May 8, 2018 •

edited by azuresdkci

Loading

AutorestCI commented May 8, 2018 •

edited

Loading

AutorestCI commented May 15, 2018 •

edited

Loading

AutorestCI commented May 15, 2018 •

edited

Loading

AutorestCI commented May 15, 2018 •

edited

Loading