Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Hub Generated] Review request for Microsoft.CostManagement to add version preview/2019-04-01-preview #5956

Conversation

bgsky
Copy link
Contributor

@bgsky bgsky commented May 14, 2019

If you are a MSFT employee you can view your work branch via this link.

Contribution checklist:

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@AutorestCI
Copy link

AutorestCI commented May 14, 2019

Automation for azure-sdk-for-js

Nothing to generate for azure-sdk-for-js

@AutorestCI
Copy link

AutorestCI commented May 14, 2019

Automation for azure-sdk-for-ruby

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

@AutorestCI
Copy link

AutorestCI commented May 14, 2019

Automation for azure-sdk-for-python

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

@AutorestCI
Copy link

AutorestCI commented May 14, 2019

Automation for azure-sdk-for-go

Nothing to generate for azure-sdk-for-go

@adxsdknet
Copy link

adxsdknet commented May 14, 2019

@bgsky
Copy link
Contributor Author

bgsky commented May 14, 2019

@praries880 @ravbhatnagar @NelsonDaniel @gahorowi This PR introduces 3 APIs: view, checkNameAvailability, and budget, by merging two earlier ones, #5895 and #5881.

Please let us know if any comments to help schedule an API review meeting.

@praries880 praries880 added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label May 14, 2019
@AutorestCI
Copy link

AutorestCI commented May 15, 2019

Automation for azure-sdk-for-java

Encountered a Subprocess error: (azure-sdk-for-java)

Command: ['/usr/local/bin/autorest', '/tmp/tmpe3zpd26h/rest/specification/cost-management/resource-manager/readme.md', '--perform-load=false', '--swagger-to-sdk', '--output-artifact=configuration.json', '--input-file=foo', '--output-folder=/tmp/tmplqv0d9_2']
Finished with return code 7
and output:

AutoRest code generation utility [version: 2.0.4283; node: v8.12.0]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
Failure:
Error: Unable to start AutoRest Core from /root/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core
Error: Unable to start AutoRest Core from /root/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core
    at main (/opt/node_modules/autorest/dist/app.js:232:19)
    at <anonymous>

/root/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core/dist/app.js:33
    autorest_core_1.Shutdown();
    ^
ReferenceError: autorest_core_1 is not defined
    at process.on (/root/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core/dist/app.js:33:5)
    at emitOne (events.js:121:20)
    at process.emit (events.js:211:7)
    at process.emit (/node_modules/source-map-support/source-map-support.js:439:21)
fs.js:612
  return binding.close(fd);
                 ^

Error: EBADF: bad file descriptor, close
    at Object.fs.closeSync (fs.js:612:18)
    at StaticVolumeFile.shutdown (/opt/node_modules/autorest/dist/static-loader.js:352:10)
    at StaticFilesystem.shutdown (/opt/node_modules/autorest/dist/static-loader.js:406:17)
    at process.exit.n [as exit] (/opt/node_modules/autorest/dist/static-loader.js:169:11)
    at printErrorAndExit (/node_modules/source-map-support/source-map-support.js:423:11)
    at process.emit (/node_modules/source-map-support/source-map-support.js:435:16)
    at process._fatalException (bootstrap_node.js:391:26)

adxsdknet added a commit to adxsdknet/azure-sdk-for-net that referenced this pull request May 22, 2019
REST Spec PR 'Azure/azure-rest-api-specs#5956'
REST Spec PR Author 'bgsky'
REST Spec PR Last commit
@bgsky
Copy link
Contributor Author

bgsky commented May 24, 2019

@ravbhatnagar @praries880 @RyanBensonMSFT Following API review with Ryan, we updated Swagger files with following changes:

  1. Remove CheckNameAvailability
  2. Remove batch deletion of views
  3. Remove version and query version in view properties

Please let us know if any comments, as we would like it signed off to onboard manifest.

adxsdknet added a commit to adxsdknet/azure-sdk-for-net that referenced this pull request May 24, 2019
REST Spec PR 'Azure/azure-rest-api-specs#5956'
REST Spec PR Author 'bgsky'
REST Spec PR Last commit
@sanjaiganesh
Copy link
Contributor

@bgsky Two asks to help review this PR

  1. Add description on the changes from previous api version
  2. Add one iteration with old api version contents and then another iteration with new api version so that we can see the diff.. Thanks.

@bgsky
Copy link
Contributor Author

bgsky commented Jun 3, 2019

@sanjaiganesh All the contents are new in this PR. No contents in old API version are included as there exist a few validation failures and we don't want conflicts as part of this PR.

To help understand PR, it introduces two new APIs, budget and view, following CRUD operations

"description": "User input name of the view. Required.",
"type": "string"
},
"scope": {
Copy link
Contributor

@ryansbenson ryansbenson Jun 3, 2019

Choose a reason for hiding this comment

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

How is scope different than the Id of the resource? Your examples also just set this to empty string so I can't really follow what this is for.

No change on scope. It is part of resource id, and empty if the view is private/at tenant level. We would like scope in resource properties to pass to another resource, so it appears in response and user does not specify it in request body(included in request url). Please refer to an example of viewByResourceGroup as a use case.

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jun 3, 2019

SDK Automation [Logs] (Generated from 292ab85)

Pending Go: Azure/azure-sdk-for-go
  • Package generation pending.
Pending Python: Azure/azure-sdk-for-python
  • Package generation pending.
Pending Ruby: Azure/azure-sdk-for-ruby
  • Package generation pending.

@bgsky
Copy link
Contributor Author

bgsky commented Jun 3, 2019

@RyanBensonMSFT We updated spec and examples with the following parts:

  1. Add x-ms-enum to Accumulated
  2. Add no content 204 in case of deletion
  3. No change on scope. It is part of resource id, and empty if the view is private/at tenant level. We would like scope in resource properties to pass to another resource, so it appears in response and user does not specify it in request body(included in request url). Please refer to an example of viewByResourceGroup as a use case.

Copy link
Contributor

@KrisBash KrisBash left a comment

Choose a reason for hiding this comment

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

ARM review feedback appears to be resolved. Thanks

@KrisBash KrisBash added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Jun 4, 2019
@praries880 praries880 merged commit a2e744b into Azure:master Jun 4, 2019
mccleanp pushed a commit that referenced this pull request Mar 23, 2022
* Adding image validation status to fidalgo machine definition.

* Add header in examples because validation says so
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants