openeemeter / eeweather

Fetch NCDC ISD, TMY3, or CZ2010 weather data that corresponds to ZIP Code Tabulation Areas or Latitude/Longitude.

http://eeweather.openee.io/

Apache License 2.0

50 stars 19 forks source link

Be more direct about cache insert / update #54

Closed craigmaloney closed 5 years ago

craigmaloney commented 5 years ago

Your checklist for this pull request

Please review the guidelines for contributing to this repository.

[x] Make sure you are requesting to pull a feature/bugfix branch (right side). Don't request your master!
[x] Make sure tests pass and coverage has not fallen docker-compose run --rm test.
[x] Update the CHANGELOG.md to describe your changes in a bulleted list under the "Development" section at the top of the changelog. If this section does not exist, create it.
[x] Make sure code style follows PEP 008 using docker-compose run --rm blacken.

Description

This resolves an issue in multi-process environments where multiple workers can retrieve a record and then try to insert them at the same time. This tries an insert first and then updates the record if the insertion fails. The difference between this and existing code is it doesn't poll for the key first (which might have changed in the interim).

codecov[bot] commented 5 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@f468948). Click here to learn what that means. The diff coverage is 100%.

@@           Coverage Diff           @@
##             master    #54   +/-   ##
=======================================
  Coverage          ?   100%           
=======================================
  Files             ?     32           
  Lines             ?   2653           
  Branches          ?      0           
=======================================
  Hits              ?   2653           
  Misses            ?      0           
  Partials          ?      0

Impacted Files	Coverage Δ
eeweather/cache.py	`100% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f468948...9b07419. Read the comment docs.

philngo commented 5 years ago

Seems like a great idea to me. I think that was the only place the key_exists function was being used, so we may also want to strip that out? Not sure about that. Regardless, this looks to me to be ready for a merge.

craigmaloney commented 5 years ago

The "key_exists" function could still be useful outside of context to query if it exists. I know we use it as a kind of cache primer on our end to ensure that the cache exists (we test to see if an invalid key is in there). There could be other unexpected uses of that function out there. :grin:

philngo commented 5 years ago

Sounds good. Released as eeweather==0.3.20. Thanks again!