Change WeatherAgent to use Openweather instead of DarkSky (which is EOL) #2848
Conversation
…ing' onecall API (which only partially matches, but gets a lot right)
…t a happy circumstance of the weather data in the fixture
|
This looks really good. Maybe we wait until Aug 1 to merge? |
Sure! I just wanted to see whether I needed to do this differently or if I was on the right track. (What's happening Aug 1?) |
|
I think that's the end of the Darksky API, according to your validation error? |
|
Oh, duh, right on. 👍 |
|
I agree in general the code looks good. What is a bit unclear to me is the date when the API is shut down. The Dark Sky blog says that it will still be available until the end of 2021. I don't have an API key myself, so I can not verify that. If their API is really still working I would love to see this change as an addition to DarkSky until they really shut down to keep the functionality for existing users. |
|
Yeah, you're right. The Android app were to be shutdown on August 1; the API is still functioning. So not sure what you wanna do. I don't see the point in creating a separate agent for 4 months; perhaps just let the PR sit until Jan 1? Not gonna bother me. What do you guys wanna do? |
|
I agree, we should wait to remove the Agent until the API is dead so that we don't break it for people. |
|
Can we add OpenWeather without removing the other APIs for now? |
That would be great,
That is also fine with me if you don't feel like investing more time into the change. |
|
Dark Sky registrations are disabled. Am I supposed to wait until the end of 2021 to implement Weather events? |
|
No, you can add it! Just don't delete support for the older APIs while they still function. |
@erebor As far as I know, the API is being shut down at the end of 2021, not Jan. 1, 2021. I think it would be worth it to either create a separate agent or provide an option in the Weather agent to switch between the two APIs. Otherwise, it means I can't use the Weather agent at all until 2022... |
@hmnd I would not mind at all! I just ran out of available time to spend on it, but I'm happy to see somebody else pick it up and run with it. |
|
@hmnd Any progress on this? It would be a good idea if the agent could support multiple API options |
|
For historical context, the weather agent originally supported two API providers, DarkSky and Weather Underground. When Weather Underground decommissioned their API I removed support for WU from the agent. |
|
FYI I'm working on updating this diff to support both Dark Sky and OpenWeather, at least until Dark Sky's API is decommissioned. I believe I've merged the two together, but need to sort out the tests as I'm not really a Ruby developer. Progress here: https://github.com/JohnMaguire/huginn/tree/openweather |
- Updates the README clarifying support of OpenWeather, Dark Sky, and Wunderground. - Merges pull request huginn#2848's support for OpenWeather in, without removing support for Dark Sky as the original pull request did. (Thanks Ryan Waldron for this!) Choosing which weather provider to use is done via the `service` option which was previously used in the WeatherAgent to support Wunderground and Dark Sky simultaneously, until Wunderground ended support for their API, at which time it was removed. Since the WeatherAgent didn't require a `service` option for a while, this diff takes the approach of defaulting to "forecastio" if no `service` option is provided in order to avoid breaking configurations prior to this change. However, the default `service` value for a new configuration will be "openweather." Additionally, "forecastio" can be explicitly specified, again to ensure compatibility with even older configurations. A note on testing - I am not a Ruby developer, so I am not quite sure how best to add tests around the branching behavior that occurs in the `model` method. Even worse, since I do not have a Dark Sky API key, I cannot manually test it. I did manually test OpenWeather. It would also be ideal to have tests which verify that the translation of data occurs without error using fixtures from each provider. Tests of this behavior did not already exist in `weather_agent_spec.rb`, however OpenWeather is tested indirectly by `agent_spec.rb` among other specs. You can find these tests easily by searching for the OpenWeather data fixture's file name ("openweather.json"). If someone were interested in creating tests of this nature in `weather_agent_spec.rb`, you can also find a fixture file for Dark Sky at `/spec/data_fixtures/weather.json` in any commit prior to the commits from huginn#2848 (starting with a61924f.)
| if key_setup? | ||
| ForecastIO.api_key = interpolated['api_key'] | ||
| onecall_endpoint = "http://api.openweathermap.org/data/2.5/onecall" |
There was a problem hiding this comment.
It sounds like new OpenWeatherMap account cannot use this API anymore. Do you think we can use https://api.openweathermap.org/data/2.5/weather instead? Doc: https://openweathermap.org/current
|
We can also just delete the DarkSky code since the API is gone now. |
This is one option for fixing #2732 . Openweather offers a free tier that's pretty generous, and they also have done some work to making porting from DarkSky reasonably easy. It's not a perfect match, but it works pretty well.
All this PR does is remove the ForecastIO gem (which is no longer maintained and no longer needed without DarkSky) in favor of just hitting the API directly with HTTParty. There are some Openweather gems, but they're pretty rudimentary, and don't buy us much that I could see.
Changed some documentation, hopefully cleared up some errors that had crept in. Comments and suggestion welcome. I'm trying to get familiar with Huginn.