azure-sdk-for-net icon indicating copy to clipboard operation
azure-sdk-for-net copied to clipboard

Published 20 hours ago • verified publisher icon Azure

Genereate and implement Maps Search V2

Open jecmenicanikola opened this issue 1 year ago • 2 comments
trafficstars

This PR contains:

  • Generated code for Maps Search from stable/2023-06-01/search.json
  • Implementation of custom models
  • Updated samples
  • Updated tests
  • Upgrade version to 2.0.0-beta.1

PR is blocked by: https://dev.azure.com/msazure/One/_git/Maps-Shared-Platform/pullrequest/9930761

jecmenicanikola avatar Apr 19 '24 18:04 jecmenicanikola

Thank you for your contribution @jecmenicanikola! We will review the pull request and get back to you soon.

github-actions[bot] avatar Apr 19 '24 18:04 github-actions[bot]

API change check

APIView has identified API level changes in this PR and created following API reviews.

Azure.Maps.Search

azure-sdk avatar May 07 '24 09:05 azure-sdk

Another important option we need to support is Accept-Language header (See API: https://learn.microsoft.com/en-us/rest/api/maps/search/get-geocoding?view=rest-maps-2024-04-01&tabs=HTTP#request-headers). In previous API, it's a query parameter, but now we need to set language from header. We need to support SearchLanguage (we should rename it to GeocodingLanguage), make it an geocoding option/reverse geocoding option, and set it to header by our own.

dubiety avatar Jun 14 '24 05:06 dubiety

@dubiety As for Accept-Language, in SearchRestClient there is logic to set that header, and during creation of SearchRestClient _acceptLanguage can be set. I am not sure that I understand what you want.

jecmenicanikola avatar Jun 19 '24 00:06 jecmenicanikola

@jecmenicanikola , MapsSearchClient.cs is where SearchRestClient() is called, if you check the place where SearchRestClient() is called, the acceptLanguage argument is always set to null or ignored.

After looking into the code, I figured out Accept-Language option cannot be set in each geocoding call, and user needs to decide it when MapsSearchClient is instentiated. In this case, we need to add a string SearchLanguage class member (don't rename it to GeocodingLanguage) in MapsSearchClientOptions.cs and set SearchLanguage in MapsSearchClient.cs when option is passed to SearchSearchClient() constructor, like L#67, L#100, and L#131. Does that make sense to you?

dubiety avatar Jun 19 '24 12:06 dubiety

Hi @jsquire,

We've fixed all the comments. Can you take a look again? Much appreciated! :)

dubiety avatar Jul 28 '24 13:07 dubiety

Hi @jsquire,

We've fixed all the comments. Can you take a look again? Much appreciated! :)

My blocker has been removed. You should be able to move forward to merge with your approval.

jsquire avatar Jul 29 '24 14:07 jsquire