palewire / savepagenow

A simple Python wrapper and command-line interface for archive.org’s "Save Page Now" capturing service
https://palewi.re/docs/savepagenow/
MIT License
167 stars 23 forks source link

bug fix: capture_or_cache was potentially making two calls to archive… #24

Closed dportabella closed 3 years ago

dportabella commented 4 years ago

bug fix: capture_or_cache was potentially making two calls to archive.org. Moreover, the second call would always return captured=False even if this second call resulted in a fresh capture

palewire commented 4 years ago

I can see your point here about the second call potentially returning a misleading response if the second shot at the API does in fact result in a fresh archive (where the first shot failed.)

However, I'm not sure I understand why you think it's necessary to remove the function entirely.

dportabella commented 4 years ago

However, I'm not sure I understand why you think it's necessary to remove the function entirely.

because it is not necessary to call requests.get twice. It's enough call it only once. if it returns a cached result, and the user don't one a cached result, you raise a CachedPage exception. if it returns a cached result, and the user accepts a cached result, you return this first response. no need to call requests.get again.

dportabella commented 4 years ago

In case you want to use this fix, I've rebased my fix with your last commits, and fixed the trailing space travis was complaining about. it seems travis is still complaining, but I don't see the actual problem.

palewire commented 3 years ago

This guy has gotten a bit stale and has a merge conflict. That's almost certainly due to me trying to debug some stuff going on with Wayback. If you want to clean up and resubmit, it would be welcome but in the interest of tidying up this neglected repo I'm going to close it for now.