Skip to content

[Ready to MERGE] Swagger for Tenant Properties API #5038

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.

Sign up for GitHub

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

Merged
praries880 merged 13 commits into from
Feb 22, 2019
Merged

[Ready to MERGE] Swagger for Tenant Properties API #5038

Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
5d3784a
added swagger for Tenant Properties
Jan 15, 2019
38018ad
Fixed camel casing in billing.json file
Jan 15, 2019
c81e213
Update billing.json
Jan 15, 2019
33d848a
changes in DiscoverTenants contracts
Jan 15, 2019
5c877c2
billing.Json trim blank space
Jan 15, 2019
c736ede
Update billing.json
Jan 15, 2019
7a96d09
removal of plural name
Jan 15, 2019
eab3b0f
Updating the route
Jan 17, 2019
b6137f4
Moved Swagger for Discover Tenants from billing to consumption
Jan 18, 2019
53fb8f8
Change in Namespace followed by one additional parameter "billingAcco…
Jan 23, 2019
b405815
Change of method name from discoverTenants to tanants.
Jan 24, 2019
42f98d2
changes ion response type
Jan 24, 2019
0d55ad6
added name after Id. Code review implementation
Jan 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Update billing.json
  • Loading branch information
Santosh Sinha committed Jan 15, 2019
commit c81e2130e29fae03a78cb1bfec7529ea6510ae1e
4 changes: 2 additions & 2 deletions ...cation/billing/resource-manager/Microsoft.Billing/preview/2018-03-01-preview/billing.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,10 @@
}
}
},
"/providers/Microsoft.Billing/DiscoverTenants?billingProfileId={billingProfileId}": {
"/providers/Microsoft.Billing/discoverTenants?billingProfileId={billingProfileId}": {
Copy link
Contributor

@KrisBash KrisBash Jan 16, 2019

Choose a reason for hiding this comment

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

As billingProfileId is mandatory, this is likely better served by a POST action request where the billingProfileId is passed in the request body. See this for an example

Copy link
Contributor Author

@santoshsinha100 santoshsinha100 Jan 16, 2019

Choose a reason for hiding this comment

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

This API is designed for Power BI custom connector to achieve multi-tenancy. This design is approved by connector Team. It is a request flow which brings Tenant Id for a specified billingProfileId input. API is doing a tenant discovery and that's why it is named as "DiscoverTenant". I hope this clarifies your question.

Copy link
Contributor

@KrisBash KrisBash Jan 16, 2019

Choose a reason for hiding this comment

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

This URL is not compliant with the Resource Provider Contract. It behaves as an action (POST) but is expressed as if it were a collection enumeration. To make this compliant with the RPC, you can model this as a POST/action, or this could be expressed as a normal collection enumeration:
`/providers/Microsoft.Billing/billingProfiles/{billingProfileId}/tenants/

Copy link
Contributor Author

@santoshsinha100 santoshsinha100 Jan 17, 2019

Choose a reason for hiding this comment

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

Changes in swagger are committed to making the URL compliant with the Resource Provider Contract. The new URL is
'/providers/Microsoft.Billing/billingProfiles/{billingProfileId}/discoverTenants'

I am keeping the name discoverTenants as the sole purpose of this API is to discover the Tenants only. As all the review comments are implemented can you please mark this approve.

Copy link
Contributor

@KrisBash KrisBash Jan 17, 2019

Choose a reason for hiding this comment

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

thank you for these changes. This path is syntactically correct. The resource type name in this case would be "discoverTenants" which is a bit odd, as verbs are atypical in a resource type name. I would prefer it to be named with a noun pattern for consistency. discoveredTenants would be substantially preferable in my opinion.

Copy link
Contributor Author

@santoshsinha100 santoshsinha100 Jan 24, 2019

Choose a reason for hiding this comment

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

I have done changes in resource type name from "discoverTenants" to "tenants" now.

"get": {
"tags": [
"TenantProperties"
"discoverTenants"
],
"x-ms-examples": {
"TenantPropertiesGet": {
Expand Down