I might, I'll look at the internals later cause at a glance I didn't really get the logic inside get_local_copy
... the if
there is ending with if ... not cached_file: return cached_file
which from reading doesn't make much sense
Legit, if you have a cached_file (i.e. exists and accessible), you can return it to the caller
BTW is the if not cached_file: return cached_file
is legit or a bug?
-_- why there isn't a link to source on the docs?
In the larger context I'd look on how other object stores treat similar problems, I'm not that advanced in these topics.
But adding a simple force_download
flag to the get_local_copy
method could solve many cases I can think of, for example I'd set it to true in my case as I don't mind the times it will re-download when not necessary as it is quite small (currently I always delete the local file, but it looks pretty ugly)
WackyRabbit7
Long story short, yes, only by name (hashing might be too slow on large files)
The easiest solution, if the hash is incorrect, delete the local copy it returns, and ask again, it will download it.
I'm not sure if the hashing is exposed, but if it is not, we can add it.
What do you think?
are you referring to the same line? 47 in cache.py?
Do you want to PR it? should be a quick fix
I mean usually it would read if cached_file: return cached_file
But adding a simple
force_download
flag to the
get_local_copy
That's sounds like a good idea
Legit, if you have a cached_file (i.e. exists and accessible), you can return it to the caller
I agree, so shouldn't it be if cached_file: return cached_file
instead of if not cached_file: return cached_file
That is actually what you are looking for:
https://github.com/allegroai/trains/blob/04b3fa809bb73d7101d1995327684ebe5b2911e3/trains/storage/cache.py#L46
We should probably change it so it is more human readable 🙂
Well I guess you can say this is definitely not self explanatory line 😉
but, it is actually asking whether we should extract the code, think of it as:if extract_archive and cached_file: return cls._extract_to_cache(cached_file, name)
nope - I'm talking about here