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

Billing RP spec changes for GTM FieldLed-Go-Big #6661

Merged
merged 33 commits into from
Aug 6, 2019
Merged

Billing RP spec changes for GTM FieldLed-Go-Big #6661

merged 33 commits into from
Aug 6, 2019

Conversation

asarkar84
Copy link
Contributor

@asarkar84 asarkar84 commented Jul 18, 2019

Latest improvements:

MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.

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.

ARM API Review Checklist

  • Service team MUST add the "WaitForARMFeedback" label if the management plane API changes fall into one of the below categories.
  • adding/removing APIs.
  • adding/removing properties.
  • adding/removing API-version.
  • adding a new service in Azure.

Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.

  • If you are blocked on ARM review and want to get the PR merged urgently, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
    Please follow the link to find more details on API review process.

Added spec for Recurring charges api
Renamed recurring charges to recurring products
Updated the billing frequency type to string
Billing RP spec changes for GTM FieldLed-Go-Big
@asarkar84 asarkar84 requested a review from wilcobmsft as a code owner July 18, 2019 02:27
@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jul 18, 2019

In Testing, Please Ignore

[Logs] (Generated from 1527d6f, Iteration 33)

In-Progress Python: test-repo-billy/azure-sdk-for-python [Logs] [Diff]
  • In-Progress azure-mgmt-billing [Logs]
Warning Java: test-repo-billy/azure-sdk-for-java [Logs] [Diff]
  • Warning billing/resource-manager/v2017_02_27_preview [Logs]
  • Warning billing/resource-manager/v2017_04_24_preview [Logs]
  • Warning billing/resource-manager/v2018_03_01_preview [Logs]
  • Warning billing/resource-manager/v2018_11_01_preview [Logs]
  • Warning billing/resource-manager/v2019_10_01_preview [Logs]
Warning Go: test-repo-billy/azure-sdk-for-go [Logs] [Diff]
  • Warning preview/billing/mgmt/2017-02-27-preview [Logs]
  • Warning preview/billing/mgmt/2017-04-24-preview [Logs]
  • Warning preview/billing/mgmt/2018-03-01-preview [Logs]
  • Warning preview/billing/mgmt/2018-11-01-preview [Logs]
Warning JavaScript: test-repo-billy/azure-sdk-for-js [Logs] [Diff]
  • Warning @azure/arm-billing [Logs]
Warning Ruby: test-repo-billy/azure-sdk-for-ruby [Logs] [Diff]
  • No packages generated.

@AutorestCI
Copy link

AutorestCI commented Jul 18, 2019

Automation for azure-sdk-for-java

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

Command: ['/usr/local/bin/autorest', '/tmp/tmpdd2hr_yz/rest/specification/billing/resource-manager/readme.md', '--azure-libraries-for-java-folder=/tmp/tmpdd2hr_yz/sdk', '--java', '--multiapi', '--use=@microsoft.azure/autorest.java@2.1.85', '--verbose']
Finished with return code 1
and output:

AutoRest code generation utility [version: 2.0.4283; node: v10.15.3]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
   Loading AutoRest core      '/root/.autorest/@microsoft.azure_autorest-core@2.0.4383/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4383)
   Loading AutoRest extension '@microsoft.azure/autorest.java' (2.1.85->2.1.85)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.38->2.3.38)
Processing batch task - {"tag":"package-2019-10-preview"} .
FATAL: System.Exception: Duplicate File Generation: src/main/java/com/microsoft/azure/management/billing/v2019_10_01_preview/implementation/AddressInner.java
   at AutoRest.Core.CodeGenerator.<Write>d__13.MoveNext() in /home/vsts/work/1/s/autorest.common/src/CodeGenerator.cs:line 151
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at AutoRest.Core.CodeGenerator.<Write>d__12.MoveNext() in /home/vsts/work/1/s/autorest.common/src/CodeGenerator.cs:line 121
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at AutoRest.Java.Azure.Fluent.CodeGeneratorJvaf.<Generate>d__6.MoveNext() in /home/vsts/work/1/s/src/azurefluent/CodeGeneratorJvaf.cs:line 188
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at AutoRest.Java.Program.<ProcessInternal>d__3.MoveNext() in /home/vsts/work/1/s/src/Program.cs:line 114
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at NewPlugin.<Process>d__20.MoveNext() in /home/vsts/work/1/s/autorest.common/src/Plugins/NewPlugin.cs:line 163
FATAL: java/generate - FAILED
FATAL: Error: Plugin java reported failure.
Process() cancelled due to exception : Plugin java reported failure.
Failure during batch task - {"tag":"package-2019-10-preview"} -- Error: Plugin java reported failure..
  Error: Plugin java reported failure.

@AutorestCI
Copy link

AutorestCI commented Jul 18, 2019

Automation for azure-sdk-for-python

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

@AutorestCI
Copy link

AutorestCI commented Jul 18, 2019

Automation for azure-sdk-for-go

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

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

Rolled back the changes in 2018-11-01-preview version
@msft-adrianma msft-adrianma added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Jul 18, 2019
@asarkar84
Copy link
Contributor Author

asarkar84 commented Jul 19, 2019

Updated action listInvoiceSectionsWithCreateSubscriptionPermission from GET to POST in this new iteration.

Incorporated review comments
Updated autorenew request params
Updated types for payment methods and available balance
Updated invoice section list examples
Updated pricesheet download path
Updated type for Invoice and Policy
Updated namespace for agreements
@majastrz
Copy link
Member

majastrz commented Aug 1, 2019

  "post": {

POST on a collection URI in ARM is considered an anti-pattern. Is it possible to use PUT to create a billing role assignment on the billing account? If not, the alternative would be to have a POST action called createBillingRoleAssignment on a billingAccount resource. #Closed


Refers to: specification/billing/resource-manager/Microsoft.Billing/preview/2019-10-01-preview/billing.json:3346 in 18ddcc3. [](commit_id = 18ddcc3, deletion_comment = False)

@majastrz
Copy link
Member

majastrz commented Aug 1, 2019

  "post": {

Same collection POST issue here. #Closed


Refers to: specification/billing/resource-manager/Microsoft.Billing/preview/2019-10-01-preview/billing.json:3431 in 18ddcc3. [](commit_id = 18ddcc3, deletion_comment = False)

@majastrz
Copy link
Member

majastrz commented Aug 1, 2019

  "post": {

Same collection POST issue here. #Closed


Refers to: specification/billing/resource-manager/Microsoft.Billing/preview/2019-10-01-preview/billing.json:3519 in 18ddcc3. [](commit_id = 18ddcc3, deletion_comment = False)

@majastrz
Copy link
Member

majastrz commented Aug 1, 2019

      "enum": [

You will need to specify the x-ms-enum extension. Also, you will need to use the modelAsString: true setting to make the linter happy. #Closed


Refers to: specification/billing/resource-manager/Microsoft.Billing/preview/2019-10-01-preview/billing.json:4311 in 18ddcc3. [](commit_id = 18ddcc3, deletion_comment = False)

@majastrz
Copy link
Member

majastrz commented Aug 1, 2019

      "enum": [

This applies to all the enums in here.


In reply to: 517489699 [](ancestors = 517489699)


Refers to: specification/billing/resource-manager/Microsoft.Billing/preview/2019-10-01-preview/billing.json:4311 in 18ddcc3. [](commit_id = 18ddcc3, deletion_comment = False)

@majastrz
Copy link
Member

majastrz commented Aug 1, 2019

"swagger": "2.0",

A few of the CI checks are failing. You'll need to fix the failures (the assigned SDK reviewer can help).


Refers to: specification/billing/resource-manager/Microsoft.Billing/preview/2019-10-01-preview/billing.json:2 in 18ddcc3. [](commit_id = 18ddcc3, deletion_comment = False)

@majastrz majastrz added the ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review label Aug 1, 2019
Copy link
Member

@majastrz majastrz left a comment

Choose a reason for hiding this comment

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

I added a few comments for the remaining issues.

Also, there are a lot of breaking changes in the API. I assume this is intentional, but how are you planning to deal with that? (If it was discussed with prior reviewers, I don’t have that context ☹.)

@asarkar84
Copy link
Contributor Author

      "enum": [

You will need to specify the x-ms-enum extension. Also, you will need to use the modelAsString: true setting to make the linter happy.

Refers to: specification/billing/resource-manager/Microsoft.Billing/preview/2019-10-01-preview/billing.json:4311 in 18ddcc3. [](commit_id = 18ddcc3, deletion_comment = False)

Sure. Done.

Updated BillingRoleAssignments actions
@asarkar84
Copy link
Contributor Author

asarkar84 commented Aug 3, 2019

@majastrz I have incorporated the comments provided. Please take a look.

Regarding BillingRoleAssignments actions in List API, I have removed the POST operation from the List API and created a new path with createBillingRoleAssignment action. We had this in our previous version and the current change was done as part of improvement. We will attempt this change again when we have support for PUT operation in our platform in future.

Added Transfer apis for partner led
Removing listInvoiceSectionsWithCreateSubscriptionPermission
@majastrz majastrz added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Aug 4, 2019
Copy link
Member

@majastrz majastrz left a comment

Choose a reason for hiding this comment

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

Signed off from ARM side.

@lirenhe lirenhe merged commit b6967e4 into Azure:master Aug 6, 2019
leni-msft pushed a commit to leni-msft/azure-rest-api-specs that referenced this pull request May 13, 2022
* API updates

* run prettier

* Remove old examples

* more changes

* update property name

* update readme.go.md

Co-authored-by: ArcturusZhang <dapzhang@microsoft.com>
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.

6 participants