Update weather API to Tomorrowio #689
Merged
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
compwron
reviewed
Oct 25, 2023
backend/Gemfile
Outdated
| # wrapper for forecast.io API, Dark Sky | ||
| gem "forecast_io" | ||
| # wrapper for tomorrow.io API | ||
| gem "tomorrowio_rb" |
compwron
reviewed
Oct 25, 2023
| @@ -1,6 +1,6 @@ | |||
| class WeatherRetriever | |||
| class << self | |||
| def get(date, postal_code) | |||
| def get(date, latitude, longitude) | |||
There was a problem hiding this comment.
ncie to not have to go thru postal code for this api
There was a problem hiding this comment.
doesn't this break the code below? the method expects postal_code in a few places
There was a problem hiding this comment.
Whoops, yeah I meant to remove postal_code in those places and replace it with coordinates.
compwron
reviewed
Oct 25, 2023
|
|
||
| if the_day.blank? | ||
| Rails.logger.warn "No forecast found for position #{position.inspect}: #{forecast.inspect}" | ||
| if forecast.status != 200 |
compwron
reviewed
Oct 25, 2023
| forecast | ||
| end | ||
|
|
||
| def create_weather(the_day, position_id) | ||
| def create_weather(forecast, position_id) | ||
| today = forecast.body["timelines"]["daily"][0] |
There was a problem hiding this comment.
nit: maybe use "dig" for json safety?
There was a problem hiding this comment.
Thanks, I'm pretty unfamiliar with Ruby and did not know about dig, I will look into that further and make some changes.
compwron
reviewed
Oct 25, 2023
| ) | ||
| end | ||
|
|
||
| def get_icon_legacy(today) | ||
| # Our icons do not coverage their full range of weather codes. We could pull in their icons (linked below) on the frontend to expand options |
Postal code is non nullable in the DB
|
I am excited for this to go thru! |
compwron
approved these changes
Jan 4, 2024
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.
Resolves #535
Context
The Dark Sky API has been completely shut down for some time now so we are migrating to Tomorrow.io to bring back weather data for users.
Approach
I first wrote a small wrapper for the API in Ruby and created a gem for that. This PR removes the old Dark Sky gem and adds my new gem and refactors the weather retriever service to use the new API. By and large the data is mostly backwards compatible with a few exceptions noted below. Notably, it was not necessary to change the Geolocation API since that is still in use. There were some previous reports of getting the wrong weather data for a given location, so it's possible we may still want to replace the Geolocation API (possibly with the browser geolocation API if that continues to be a problem.
Exceptions to backwards compatibility
Tests
Tests to follow, I need to familiarize myself with the VCR package being used here as well as generate the requisite API data for that. Just want to put up an initial draft PR to get some feedback on this so far.
Other resources
Expand for API translations that I came up with