Update weather agent to support both Dark Sky and OpenWeather #2900
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ing' onecall API (which only partially matches, but gets a lot right)
…t a happy circumstance of the weather data in the fixture
johnmaguire
commented
Nov 11, 2020
| if dark_sky? | ||
| # AFAIK, there is no warning-level log messages. In any case, I'd like to log this from validate_options but | ||
| # since the Agent doesn't exist yet, the Log record can't be created (no parent_id) | ||
| log "NOTICE: The DarkSky API will be disabled at the end of 2021; please switch to OpenWeather." if dark_sky? |
There was a problem hiding this comment.
This was the reason for #2899. I am hoping this isn't too annoying.
- Updates the README clarifying support of OpenWeather, Dark Sky, and Wunderground. - Merges pull request #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 #2848 (starting with a61924f.)
|
I closed this PR because there is apparently no interest in it. |
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2732 and #2895.
Wunderground.
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
serviceoptionwhich 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
serviceoption for a while,this diff takes the approach of defaulting to "forecastio" if no
serviceoption is provided in order to avoid breaking configurationsprior to this change. However, the default
servicevalue for a newconfiguration 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
modelmethod. Even worse, since I do not have a Dark Sky API key, Icannot 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 byagent_spec.rbamong other specs. You can find these tests easily bysearching 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 Skyat
/spec/data_fixtures/weather.jsonin any commitprior to the commits from #2848 (starting with
a61924f.)