added check for cache if bad response from server #203
Conversation
Codecov Report
@@ Coverage Diff @@
## master #203 +/- ##
=========================================
+ Coverage 74.28% 74.78% +0.5%
=========================================
Files 7 7
Lines 350 353 +3
=========================================
+ Hits 260 264 +4
+ Misses 90 89 -1
Continue to review full report at Codecov.
|
| @@ -15,14 +15,14 @@ class Vulnerability(namedtuple("Vulnerability", | |||
| pass | |||
|
|
|||
|
|
|||
| def get_from_cache(db_name): | |||
| def get_from_cache(db_name, site_down=False): | |||
There was a problem hiding this comment.
The term site_down does not align with a cache thing. I mean, we are already getting from cache here. I think we could pass something like valid_seconds.
| if os.path.exists(CACHE_FILE): | ||
| with open(CACHE_FILE) as f: | ||
| try: | ||
| data = json.loads(f.read()) | ||
| if db_name in data: | ||
| if "cached_at" in data[db_name]: | ||
| if data[db_name]["cached_at"] + CACHE_VALID_SECONDS > time.time(): | ||
| if site_down or data[db_name]["cached_at"] + CACHE_VALID_SECONDS > time.time(): |
There was a problem hiding this comment.
If valid_seconds is negative, consider always valid.
| @@ -84,6 +84,10 @@ def fetch_database_url(mirror, db_name, key, cached, proxy): | |||
| return data | |||
| elif r.status_code == 403: | |||
| raise InvalidKeyError() | |||
| else: | |||
| cached_data = get_from_cache(db_name=db_name, site_down=True) | |||
There was a problem hiding this comment.
This is a good thing for other HTTP status codes, but we still need to catch requests connection exceptions as well.
| self.assertEqual(len(vulns), 1) | ||
|
|
||
| self.assertEqual(len(vulns), 1) | ||
There was a problem hiding this comment.
Remove these blanks, please?
| # the cache | ||
| vulns = safety.check( | ||
| packages=packages, | ||
| db_mirror="http://pyup.io/", |
There was a problem hiding this comment.
Can you use mock to avoid letting requests performing an actual HTTP request?
|
Hi @rafaelpivato, thank you for all your comments! I will address them shortly. Things have been a bit hectic with work and life recently. |
If the remote safety db is down (this happened on 2/15), is it reasonable to check to see if there has been anything cached locally, even if
--cacheis nottrueat the time?