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

{AzureSubscription} Fixes Azure/azure-sdk-for-go#18619 #18670

Closed
wants to merge 1 commit into from

Conversation

navba-MSFT
Copy link

@navba-MSFT navba-MSFT commented Jul 22, 2022

Fixes #18619

The TenantIDDescription struct returned by calls to TenantsClient.ListComplete contains only two fields: ID and TenantID:

However, the underlying API returns many more useful fields, including countryCode, displayName, domains, tenantCategory defaultDomain and tenantType. It would be great if these fields could also be made accessible through the Go SDK.

  • The purpose of this PR is explained in this or a referenced issue.
  • The PR does not update generated files.
  • Tests are included and/or updated for code changes.
  • Updates to CHANGELOG.md are included.
  • MIT license headers are included in each file.

Fixes #18619

The TenantIDDescription struct returned by calls to TenantsClient.ListComplete contains only two fields: ID and TenantID:

However, the underlying API returns many more useful fields, including countryCode, displayName, domains, tenantCategory defaultDomain and tenantType. It would be great if these fields could also be made accessible through the Go SDK.
@hermanschaaf
Copy link

Hi @navba-MSFT, thanks again for championing this change. I just wanted to note that I believe this is similar to the subscription case, in that the file here is generated from an underlying JSON source, so it will need to be updated at the source first.

Copy link
Member

@chlowell chlowell left a comment

Choose a reason for hiding this comment

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

This file is generated. If it needs an update, we should regenerate the package rather than edit its files manually. The code here is correct though. It was generated for a REST API whose TenantIdDescription model has only two fields, as you can see in its specification here (sorry I don't know where this API is found in the REST docs). The SDK package corresponding to the REST API mentioned in the linked issue is services/resources/mgmt/2021-01-01/subscriptions. However, that package is deprecated, so I recommend using the new armsubscriptions module instead.

This is a confusing couple of packages/APIs. Can another reviewer comment on why we have armsubscription and armsubscriptions packages?

@tadelesh
Copy link
Member

@navba-MSFT As explanation in this PR, I'll close this PR. Thanks @chlowell to help to explain the deprecation of the package. I'll check these two subscription packages.

@tadelesh tadelesh closed this Jul 25, 2022
@RickWinter RickWinter deleted the navba-MSFT-patch-2 branch March 15, 2023 18:02
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.

Tenants List should return all the fields available in the API
4 participants