download_file with cache=True uses cache unconditionally
- Dominant language
- Python
- Stars
- 5.3k
- Forks
- 2.2k
- Avg merge
- 1d 18h
- Merged PRs (30d)
- 74
Description
We should improve the caching mechanism for `download_file` so that it can still check the server (where possible) for updates to the remote file. This might also mean changing the options available for `cache` as to whether or not to use the cache unconditionally (or perhaps adding a separate option of `cache_only` where `cache_only=True` would force use of the cache where possible, and otherwise download the file).
However, when `cache=True` and `cache_only=False` (which I think should be the default), we should always check the remote server to see if the remote content has updated, by looking at a combination of the `ETag` and `Last-Modified` headers. However, it should be noted that not all servers implement these headers, and some implement them unreliably. [This paper](http://iwaw.europarchive.org/04/Clausen.pdf) describes an evidence-based scheme for best determining if these headers (when present, or not present) indicate whether a file has changed. TL;DR, they found the best scheme (in terms of reliability of correctly indicating a change, and avoiding unnecessary downloads) is as follows. If any of these conditions are true, the file can be redownloaded:
1. ETag is same, but timestamp is changed
2. ETag is changed, but timestamp is same
3. Both ETag and timestamp changed
4. ETag is changed, but timestamp is _missing_
5. ETag is _missing_, but timestamp is same (this one is surprising, but turns out to more often than not indicate a poorly configured server)
6. ETag is _missing_, but timestamp is changed
7. Both ETag and timestamp are missing
In all other cases the file can be assumed unchanged. We may wish to mix this up a bit, for example to prefer the cached copy if we can't reliably determine if the remote content has changed.
Another technique, they noted, for improving reliability is to keep track of the reliability of specific servers. For example, if a server reports the content changed (via a different Last-Modified or ETag), but the downloaded content ends up being identical to the content we already had cached, we could mark that server (via its FQDN) as unreliable. On the other hand, we can't as easily catch cases where we don't download some content because the server (wrongly) indicated that the content changed. In that case we can go ahead and use the cached copy, but if the user is definitely expecting that that file should have changed, they can switch to using `cache=False`. When using `cache=False` we should still compare downloaded data to the existing data (just the hashes, that is), to determine server reliability. So we would always keep an up-to-date flag in the cache as to which servers have reliable ETags, if nothing else.
Both the server reliability flags, and storing each URL's ETag and Last-Modified headers will necessitate a change to the download cache database. I think the new format should include a version number (where caches with a missing version can be assumed version 0--the current version). The new format could be something like:
``` python
{
'version': 1,
'servers': {
'fully.qualified.domain.name': {
'reliable-etag': True/False
# This is a dict, to allow for future server metadata
},
'files': {
'http://full.url/of/file': {
'download-dir': '/path/to/saved/file',
'hash': 'hash-string',
'hash-algorithm': (maybe?),
'etag': 'etag of cached file',
'last-modified': 'last modified timestamp of cached file'
}
}
}
```
In all cases, there should also be better log messages about how the cache is being used--when a file is being loaded from the cached, or downloaded, etc.
This might also be a good opportunity for a little code refactoring. For example, it might be nice if the caching mechanism were, itself, implemented as a context manager of some kind, though I haven't worked out the details.
Contributor guide
Research direction
Start by locating the existing download_file entry point and the download cache database implementation. Review how cache=True and cache=False currently behave, then define the cache format, server-header reliability handling, logging, and tests needed to verify conditional downloads and cache-only behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100