[Azure Maps - Azure Maps] API Review

[azure-sdk]

New API Review meeting has been requested.

Service Name: Azure Maps - Azure Maps
Review Created By: Will Huang
Review Date: 05/23/2024 08:00 AM PT
Release Plan:
PR: #29153
Hero Scenarios Link: See
Core Concepts Doc Link: See

Description: SnapToRoads/RouteRange APIs refview

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]

Notes from API Review meeting 5/23/24

• Why have a separate "/result" operation? Better to just include the result in the status monitor.
• The error example is probably a bad example, because that should be detected before return from initial request
• Azure would normally use "kind" rather than "operationType", since this property is a discriminator
• Consider making the operations endpoint specific to the matrix operation, /route/matrix/operations, so that the response does not need to be polymorphic and you won't need the operationType property.
• What is the purpose of the summary field? Is it only present when the operation is complete and successful?
  • Consider removing that field since it really should be part of the result
• What happens on "partial success"? Operation succeeds even if all routes failed.
• Recommend avoid HTTP protocol details in error messages (e.g. "POST")
• Do you need the accept-language request header ? Unusual to see this.

This looks good for preview. Please incorporate the above into the preview if possible/desirable.

[koyasu221b]

• Why have a separate "/result" operation? Better to just include the result in the status monitor.

  • It's a good idea to combine results directly into the result object instead of introducing a new API. However, considering our result size is large, the average latency of the get status API is not consistent.
  • Therefore, I think continuing to provide the /result API can enhance user experience, and our backend can also offer a streaming method for large results through the /result API.

• The error example is probably a bad example, because that should be detected before return from initial request

  • Updated the error example

• Azure would normally use "kind" rather than "operationType", since this property is a discriminator

  • After internal discussion, we will adopt Azure's naming conventions, changing operationType to kind and operationId to id

• What is the purpose of the summary field? Is it only present when the operation is complete and successful?

  • We removed this field from the OperationResponse but kept it in the RouteMatrixResponse. This field indicates how many routes have failed from origin to destination.

• Recommend avoid HTTP protocol details in error messages (e.g. "POST")

  • Agreed

• Do you need the accept-language request header ? Unusual to see this.

  • We actually don't need it for RouteMatrix; we will remove it. However, for the result API, I think we need to keep it since the accept-language is used for display language. If in the future we need to support an async RouteDirections API and the response contains instructions that need translation into another language, I think it will still be useful.

[mikekistler]

I'm not sure I understand the concern with average latency for the /operations endpoint. The response will be small and quick while the operation is still in flight. When the operation is complete, users will only need one API call rather than two to obtain the results, and whether the results are small or large the performance of one call has to be better than two.

As for streaming, I'm not sure what you mean there. HTTP allows "chunking" of the response body and this can be implemented on any response. If you actually mean pagination, then this needs to be added to "/results" now because it is a breaking change to add pagination later.

[koyasu221b]

I mean that if the result object is large, the user might have to wait about 20 seconds to get the result. However, if the result object is small, on average it only takes about 1 second. I expect that regardless of whether the status is success or failure, the user can receive the status in under 1 second. This way, they can quickly determine the current operation status and to decide if they want to download the result

[mikekistler]

I don't understand how breaking this into two operations will reduce the time to receive the result. If the result is large, it's going to take a while to transfer. Breaking this into two operations only adds to the latency.

[koyasu221b]

The result time is fixed, but the user can check the status quickly.
I expect that regardless of whether the status is success or failure, the user can receive the status in under 1 second. This allows the user to decide whether they want to download the result.

[mikekistler]

Is it a common use case to perform this operation and not want to download the result?

[koyasu221b]

The status API can be lightweight and designed for quick responses, while the download API can manage the heavier load of data transfer. If the status API takes too long, users might unexpectedly cancel, but now that we've separated them, they know their result is actually complete. Thus, they understand that their response is large and will take a long time to download. They can then tailor their logic based on their needs.