search

seperman / fast-autocomplete

Fast Autocomplete: When Elastcsearch suggestions are not fast and flexible enough
MIT License
271 stars 40 forks source link

Fixup: update cache key reference to requirements.txt (removed) #16

Closed jayaddison closed 3 years ago

jayaddison commented 4 years ago

I missed this reference to requirements.txt while developing #14; using setup.py as a dependency cache key should be relatively stable and maintains the property that the cache should be invalidated when dependencies are updated (although unfortunately it may also invalidate the cache during other unrelated changes)

May resolve #15

seperman commented 4 years ago

Hi @jayaddison Sorry I'm not sure how I missed this PR. Why do you think setup.py should be used for the signature instead of requirements.txt?

jayaddison commented 4 years ago

Hey @seperman - no problem! Using setup.py in the key isn't ideal (because it contains non-dependency-related code and content), but requirements.txt was removed in #14 (due to the dynamic dependency selection) so I don't think it's suitable as a cache key.

Given that caching is a nice-to-have (i.e. lack of cache shouldn't cause failures, although it can waste time and resources), and that setup.py shouldn't change often -- and should always change when the dependency list does -- it seemed the next best option.

jayaddison commented 3 years ago

Closing this as stale; there's a small chance it may resolve #15, but I don't know when I'd be likely to investigate that more thoroughly.

seperman commented 3 years ago

Ok thanks for closing the ticket.