Skip to content

Comments

Add Batch dataplane API version 2018-12-01.8.0#4795

Merged
anuchandy merged 4 commits intoAzure:masterfrom
matthchr:feature/batch-dataplane-2018-12
Dec 10, 2018
Merged

Add Batch dataplane API version 2018-12-01.8.0#4795
anuchandy merged 4 commits intoAzure:masterfrom
matthchr:feature/batch-dataplane-2018-12

Conversation

@matthchr
Copy link
Member

@matthchr matthchr commented Dec 5, 2018

Contribution checklist:

  • I have reviewed the documentation for the workflow.
  • Validation tools were run on swagger spec(s) and have all been fixed in this PR.
  • The OpenAPI Hub was used for checking validation status and next steps.

This is a data plane API so ARM review is not required.

I tried to run the linter here: https://portal.azure-devex-tools.com/app/tools/linter but it just hung on "still working!" forever.
I also tried to run this: https://portal.azure-devex-tools.com/app/tools/static-validation/static/errors/default pointing at https://github.com/matthchr/azure-rest-api-specs/tree/feature/batch-dataplane-2018-12/specification/batch/data-plane/Microsoft.Batch/stable/2018-12-01.8.0 and I also got an error "Sorry something went wrong!"

I did run the azure validator manually though and things looked okay (there's some warnings but they basically don't seem to apply to data-plane)

@openapi-portal-comment
Copy link

If you're a MSFT employee, click this link
to view this PR's validation status on our new OpenAPI Hub spec management tool.

@AutorestCI
Copy link

AutorestCI commented Dec 5, 2018

Automation for azure-sdk-for-js

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

@AutorestCI
Copy link

AutorestCI commented Dec 5, 2018

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

@AutorestCI
Copy link

AutorestCI commented Dec 5, 2018

Automation for azure-sdk-for-python

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-sdk-for-python#4031

@AutorestCI
Copy link

AutorestCI commented Dec 5, 2018

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

@AutorestCI
Copy link

AutorestCI commented Dec 5, 2018

Automation for azure-sdk-for-node

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-sdk-for-node#4416

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@AutorestCI
Copy link

AutorestCI commented Dec 5, 2018

Automation for azure-sdk-for-go

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

@anuchandy
Copy link
Member

@matthchr I will talk to owners of the devex tool on the issues you faced.

Could you take a look at model (example) validation https://travis-ci.org/Azure/azure-rest-api-specs/jobs/463987019?

@matthchr
Copy link
Member Author

matthchr commented Dec 5, 2018

@anuchandy - I will fix the example validation issues and update the PR

@matthchr
Copy link
Member Author

matthchr commented Dec 5, 2018

The examples should be fixed now

@matthchr
Copy link
Member Author

matthchr commented Dec 5, 2018

I fixed the original issue with the model validator, but am now getting a different one which seems very unclear to me. See: https://travis-ci.org/Azure/azure-rest-api-specs/jobs/464004216

operationId: Application_Get
scenario: Get applications
source: request
responseCode: ALL
severity: 0
errorCode: REQUIRED
errorDetails:
  code: REQUIRED
  message: Value is required but was not provided
  path: >-
    /applications/{applicationId}/applicationId/paths//applications/{applicationId}/get/parameters/0

It's not clear to me what the actual problem is...

Copy link
Contributor

Choose a reason for hiding this comment

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

@matthchr - Matt, sorry, can you please update this description to change the incorrect doc link? I suggest:
"description": "This property is mutually exclusive with other ImageReference properties. The virtual machine image must be in the same region and subscription as the Azure Batch account. For more details, see https://docs.microsoft.com/azure/batch/batch-custom-images."

Copy link
Member Author

Choose a reason for hiding this comment

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

@dlepow did you put this on the wrong line?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I found where it was meant to be though -- I've fixed the docs there

Copy link
Contributor

Choose a reason for hiding this comment

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

@matthchr - Thanks. When I commented before the GitHub UI was a bit flaky with the large file, so I might have selected the wrong place.

@matthchr matthchr force-pushed the feature/batch-dataplane-2018-12 branch from e275707 to 9b00f1b Compare December 6, 2018 19:05
@matthchr matthchr force-pushed the feature/batch-dataplane-2018-12 branch 3 times, most recently from 75930dc to 3f5b3a4 Compare December 7, 2018 18:05
@matthchr
Copy link
Member Author

matthchr commented Dec 7, 2018

@anuchandy I have confirmed that the URLs should contain HTTPS (at least, in the code generated by AutoRest you must set the URL to https://<account>.<region>.batch.azure.com). Given that, I think it makes sense to leave the examples with the HTTPS, as I am pretty sure it's supposed to be there.

Is there anything else that needs to be fixed, or is this good to merge?

@anuchandy
Copy link
Member

@sergey-shandar - do you think this is a bug in OAV? OR should "https" has to be removed from examples? we may want to set a guideline for service team here, what I understand from our hallway talk is - "the swagger scheme part says it's https and as per http convention, host name does not contain protocol." Can you please confirm?

Tracked here: Azure/oav#368

 - Add batchUrl parameter.
 - Rename "ocp-data" to "ocp-date" to correctly reflect what the API
   actually requires.
@matthchr matthchr force-pushed the feature/batch-dataplane-2018-12 branch from 3f5b3a4 to 8460714 Compare December 10, 2018 21:38
@matthchr
Copy link
Member Author

@anuchandy - I believe the examples should be fixed now, I removed HTTPS from them

@anuchandy anuchandy merged commit d8805f6 into Azure:master Dec 10, 2018
@matthchr matthchr deleted the feature/batch-dataplane-2018-12 branch December 11, 2018 21:59
TalluriAnusha pushed a commit to AsrOneSdk/azure-rest-api-specs that referenced this pull request Feb 6, 2019
* Prepare for Azure Batch new API version

* Add new data plane API version 2018-12-01.8.0

* Fix documentation issues

* Fix examples

 - Add batchUrl parameter.
 - Rename "ocp-data" to "ocp-date" to correctly reflect what the API
   actually requires.
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.

6 participants