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

[Azure Maps - Azure Maps] API Review #31596

Open
azure-sdk opened this issue Nov 18, 2024 · 7 comments · May be fixed by #31578
Open

[Azure Maps - Azure Maps] API Review #31596

azure-sdk opened this issue Nov 18, 2024 · 7 comments · May be fixed by #31578
Labels
API Review Scoping This is an issue that will track work on a specific set of API changes.

Comments

@azure-sdk
Copy link
Collaborator

New API Review meeting has been requested.

Service Name: Azure Maps - Azure Maps
Review Created By: Zhitong Xie
Review Date: 12/19/2024 04:00 PM PT
Release Plan: 1522
PR: #31578
Hero Scenarios Link: Not Provided
Core Concepts Doc Link: Not Provided

Description:

Detailed meeting information and documents provided can be accessed here
For more information that will help prepare you for this review, the requirements, and office hours, visit the documentation here

@azure-sdk azure-sdk added the API Review Scoping This is an issue that will track work on a specific set of API changes. label Nov 18, 2024
@azure-sdk azure-sdk linked a pull request Nov 18, 2024 that will close this issue
3 tasks
@azure-sdk
Copy link
Collaborator Author

Meeting updated by Zhitong Xie

Service Name: Azure Maps - Azure Maps
Review Created By: Zhitong Xie
Review Date: 12/10/2024 01:00 PM PT
Release Plan: 1522
PR: #31578
Hero Scenarios Link: Not Provided
Core Concepts Doc Link: Not Provided

Description:

Detailed meeting information and documents provided can be accessed here
For more information that will help prepare you for this review, the requirements, and office hours, visit the documentation here

1 similar comment
@azure-sdk
Copy link
Collaborator Author

Meeting updated by Zhitong Xie

Service Name: Azure Maps - Azure Maps
Review Created By: Zhitong Xie
Review Date: 12/10/2024 01:00 PM PT
Release Plan: 1522
PR: #31578
Hero Scenarios Link: Not Provided
Core Concepts Doc Link: Not Provided

Description:

Detailed meeting information and documents provided can be accessed here
For more information that will help prepare you for this review, the requirements, and office hours, visit the documentation here

@azure-sdk
Copy link
Collaborator Author

Meeting updated by Zhitong Xie

Service Name: Azure Maps - Azure Maps
Review Created By: Zhitong Xie
Review Date: 12/10/2024 01:00 PM PT
Release Plan: 1522
PR: #31578
Hero Scenarios Link: Not Provided
Core Concepts Doc Link: Not Provided

Description: https://microsoft-my.sharepoint.com/:p:/p/zhitongxie/ERDHWL_9rxpDiZ9xxE8lVK4BNFGcM26FAaiIzPd3b68dOw?e=5JIqYo

Detailed meeting information and documents provided can be accessed here
For more information that will help prepare you for this review, the requirements, and office hours, visit the documentation here

@mikekistler
Copy link
Member

Notes from API Review 12/10/2024

  • Pls consider making the response of getIncidents pageable (include nextLink)
  • Need an offline discussion on unified API versioning in Maps
  • Is this a new service ?
    • It's actually a new GA version that is a breaking change from the prior (1.0)
    • The breaking change will be communicated sometime next year
    • We need to navigate that correctly (now)
  • Question on error response documentation
    • Probably need a fix in specification/common-types/data-plane/v1/types.json
  • Why pass the x-ms-client-id? This should be taken offline with the Security board
    • Start with Scott Schaab
  • Also -- for SAS token, you should only support delegated SAS tokens

Need to resolve the versioning and security concerns before we can approve for GA.

@miaxzhitong
Copy link
Member

Hi Mike,

Here are some of our follow-ups:

  • Pls consider making the response of getIncidents pageable (include nextLink)

We have updated the PR to add 'links' section to response, the format of the links follows OGC API - Features - Part 1 standards. We will have next as one of the relations under 'links'. Please let me know if you have any suggestion here. Thanks so much!

  • Question on error response documentation. Probably need a fix in specification/common-types/data-plane/v1/types.json
    My understanding is that most of Azure services are following this error response schema (Please correct me if I am wrong). Please let me know if we should switch our reference to somewhere else. Thanks so much!

As for the versioning related questions, @FarazGIS will take care of it.

As for the security and auth related questions, @subbarayudukamma will respond later.

Thanks so much!

@FarazGIS
Copy link
Contributor

@mikekistler I sent an email this morning on versioning and breaking change.

@mikekistler
Copy link
Member

mikekistler commented Jan 24, 2025

@FarazGIS Regarding issues besides API versioning, I think there was an email thread on the pagination support. Pagination should be implemented according to Azure Guidelines, with nextLink in the response body to provide a link to the next page of results. Has this been resolved?

Update: I found the email thread. The resolution appears to be to remove pagination altogether. Has that change been implemented?

Update 2: I checked the PR and it does seem that this has been integrated, so the only outstanding concern (I know of) is the api versioning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Review Scoping This is an issue that will track work on a specific set of API changes.
Projects
Status: Triage
Development

Successfully merging a pull request may close this issue.

4 participants