Closed bytesized closed 8 years ago
@willkg Are you available to take a look at this?
Seems like you only want to retry the network download. Rather than having all that in one big function, it'd be easier to reason about to separate the network download from the rest of it and then wrap the network download code in the retry logic.
I just implemented a similar thing, but did it as a decorator which makes it easier to reason about the thing being retried separate from the retry logic.
https://github.com/mozilla/antenna/blob/master/antenna/util.py#L127
There are a bunch of retry decorator Python libs:
https://github.com/mozilla/antenna/issues/74#issuecomment-252278522
I have no idea whether those work with tornado and how it does network io.
I think I would restructure this into better parts that are easier to reason about and test.
Hmm. I'll try to look for a more elegant solution. It is a bit tricky, because the program allows for multiple servers. Each server could have the symbol file, but none are guaranteed to. On failure, it makes the most sense to try the next server rather than immediately retrying the last one. But on retry, we don't want to make requests of servers that already responded with a 404.
Could you be more specific about what changes you would like to see? Does the retry logic need to be in a decorator?
I'd split that massive function into "get the file" and a "do stuff with the file" parts first. After that, it'll be easier to see what's going on and how to better do retry logic.
@willkg Better?
@willkg Ok, now the download logic has been separated from the save logic. Does that seem better?
Addresses Issue #43 to add download retries when a download fails. If the server explicitly says that it does not have the file (HTTP 404), no retries will be attempted.