JustinCanton / Geo.NET

A lightweight method for communicating with the multiple online geocoding APIs.
MIT License
13 stars 8 forks source link

Add the possibility to configure API Keys at runtime #80

Closed ollie10 closed 5 months ago

ollie10 commented 1 year ago

Hello @JustinCanton,

I was using the library and I saw something that could improve the usability of it. Basically it seems that there is no possibility to configure API keys of the providers outside the Startup.

This could lead to some problems sometimes, maybe you want to have some custom or complex logic to decide which API key should you use directly in the service using the provider.

Basically, something like this:

public MapboxGeocodingHelpers(IOptions<ApplicationConfiguration> appSettings, IMapBoxGeocoding mapBoxGeocoding)
{
    _mapBoxGeocoding = mapBoxGeocoding;
    _mapBoxGeocoding.SetApiKey(appSettings.Value.MapboxAPIKey)
}

Or like the official Google API is doing

public async Task<CompleteAddressDTO> GeocodeAddress(string address, CultureInfo culture)
{
    AddressGeocodeRequest request = new AddressGeocodeRequest { Key = _appSettings.GoogleMapsApiKeyForBackEnd, Address = address, Language = GetLanguageFromCulture(culture) };
    GeocodeResponse response = await GoogleMaps.Geocode.AddressGeocode.QueryAsync(request);
    ...
}

Do you plan to implement something like that? I think it will be very useful in various scenarios

JustinCanton commented 1 year ago

Hello @ollie10 ,

I don't particularly have any plans to add this at the moment. Is there any use case in particular you are interested having this functionality for?

ollie10 commented 1 year ago

Hi @JustinCanton,

Perhaps easier testing and configure API keys reading from some external storage, something you wouldn't do in Startup normally

JustinCanton commented 1 year ago

The only real reason I can see to have this would be if you want to change the key between different requests.

I don't really see the API key being read from an external storage as a use case, because the external storage can be read at startup. Even the example of google uses an IConfiguration item to configure the key beforehand.

I will consider it, but it won't be very high on my priority list.

ollie10 commented 1 year ago

@JustinCanton that's it is also a scenario, in case you exhaust your key and want to use another or it fails and you have a backup one

JustinCanton commented 5 months ago

Hello @ollie10,

Sorry this took so long. I have a PR open for these changes. But they will be included with a bundle of other changes including breaking changes. If you want to use this, you will need to use the 2.0.0 package I plan on releasing by the end of this month.

To see the list of breaking changes, please checkout the changelog here.

ollie10 commented 5 months ago

Hello Justin, no worries, I'll wait version 2 and test directly that one, no rush for me many thanks for letting me know!