Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

storeapi: resume snap downloads #1330

Merged
merged 8 commits into from May 28, 2017

Conversation

Projects
None yet
4 participants
@sergiusens
Copy link
Collaborator

commented May 23, 2017

Signed-off-by: Sergio Schvezov sergio.schvezov@canonical.com

storeapi: log download retries
Signed-off-by: Sergio Schvezov <sergio.schvezov@canonical.com>
@sergiusens

This comment has been minimized.

Copy link
Collaborator Author

commented May 24, 2017

ok, seems we are doing the right thing

Starting new HTTPS connection (1): public.apps.ubuntu.com
"GET /anon/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap HTTP/1.1" 302 0
Starting new HTTPS connection (1): 068ed04f23.site.internapcdn.net
"GET /download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap?t=2017-05-24T21:12:06Z&h=d17cbd8892be4542d2a8e7bdbffa079d73001bd4 HTTP/1.1" 200 83349504
Retries left to download 'https://public.apps.ubuntu.com/anon/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap': 5.
"GET /anon/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap HTTP/1.1" 302 0
Resetting dropped connection: 068ed04f23.site.internapcdn.net
"GET /download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap?t=2017-05-24T21:12:08Z&h=72ada5b9c36d65fdef3b7beed1bef4465188d1a4 HTTP/1.1" 200 83349504
Retries left to download 'https://public.apps.ubuntu.com/anon/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap': 4.
"GET /anon/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap HTTP/1.1" 302 0
Resetting dropped connection: 068ed04f23.site.internapcdn.net
"GET /download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap?t=2017-05-24T21:12:09Z&h=be65420324c9ee8c88bdd14fa1c87afcb06478c5 HTTP/1.1" 200 83349504
Retries left to download 'https://public.apps.ubuntu.com/anon/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap': 3.
"GET /anon/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap HTTP/1.1" 302 0
Resetting dropped connection: 068ed04f23.site.internapcdn.net
"GET /download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap?t=2017-05-24T21:12:11Z&h=833102254c063dd1502a1f0f6a2fec634bed6659 HTTP/1.1" 200 83349504
Retries left to download 'https://public.apps.ubuntu.com/anon/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap': 2.
"GET /anon/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap HTTP/1.1" 302 0
Resetting dropped connection: 068ed04f23.site.internapcdn.net
"GET /download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap?t=2017-05-24T21:12:13Z&h=6418ec910dbbd9bcd7bd002c8687a069b141614b HTTP/1.1" 200 83349504
Retries left to download 'https://public.apps.ubuntu.com/anon/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap': 1.
("Connection broken: ConnectionResetError(104, 'Connection reset by peer')", ConnectionResetError(104, 'Connection reset by peer'))
@sergiusens

This comment has been minimized.

Copy link
Collaborator Author

commented May 24, 2017

I will edit the logged debug message so we get the thrown exception in there too

sergiusens added some commits May 24, 2017

@sergiusens

This comment has been minimized.

Copy link
Collaborator Author

commented May 24, 2017

Starting new HTTPS connection (1): search.apps.ubuntu.com
"GET /api/v1/snaps/details/core?channel=stable&fields=status%2Canon_download_url%2Cdownload_url%2Cdownload_sha3_384%2Cdownload_sha512%2Csnap_id%2Crevision%2Crelease HTTP/1.1" 200 573
Downloading core
Starting new HTTPS connection (1): public.apps.ubuntu.com
"GET /anon/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap HTTP/1.1" 302 0
Starting new HTTPS connection (1): 068ed04f23.site.internapcdn.net
"GET /download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap?t=2017-05-25T22:36:16Z&h=c1cda6c141defcbf04819467b55117e2a6863cff HTTP/1.1" 200 83349504
Redirections for 'https://public.apps.ubuntu.com/anon/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap': https://068ed04f23.site.internapcdn.net/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap?t=2017-05-25T22:36:16Z&h=c1cda6c141defcbf04819467b55117e2a6863cff
Error while downloading: ChunkedEncodingError(ProtocolError("Connection broken: ConnectionResetError(104, 'Connection reset by peer')", ConnectionResetError(104, 'Connection reset by peer')),). Retries left to download: 5.
"GET /anon/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap HTTP/1.1" 302 0
Resetting dropped connection: 068ed04f23.site.internapcdn.net
"GET /download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap?t=2017-05-25T22:36:24Z&h=bf88e0517fb1390ed5671c7b0ebf0612b18a9d3e HTTP/1.1" 200 83349504
Redirections for 'https://public.apps.ubuntu.com/anon/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap': https://068ed04f23.site.internapcdn.net/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap?t=2017-05-25T22:36:24Z&h=bf88e0517fb1390ed5671c7b0ebf0612b18a9d3e
Error while downloading: ChunkedEncodingError(ProtocolError("Connection broken: ConnectionResetError(104, 'Connection reset by peer')", ConnectionResetError(104, 'Connection reset by peer')),). Retries left to download: 4.
"GET /anon/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap HTTP/1.1" 302 0
Resetting dropped connection: 068ed04f23.site.internapcdn.net
"GET /download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap?t=2017-05-25T22:36:26Z&h=cbd0753f009f7d9ca8a7a4f18b1fd6032d2f04ae HTTP/1.1" 200 83349504
Redirections for 'https://public.apps.ubuntu.com/anon/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap': https://068ed04f23.site.internapcdn.net/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap?t=2017-05-25T22:36:26Z&h=cbd0753f009f7d9ca8a7a4f18b1fd6032d2f04ae
Error while downloading: ChunkedEncodingError(ProtocolError("Connection broken: ConnectionResetError(104, 'Connection reset by peer')", ConnectionResetError(104, 'Connection reset by peer')),). Retries left to download: 3.
"GET /anon/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap HTTP/1.1" 302 0
Resetting dropped connection: 068ed04f23.site.internapcdn.net
"GET /download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap?t=2017-05-25T22:36:28Z&h=81eb231649cba42085e9a5ab0ded3b756f0c9881 HTTP/1.1" 200 83349504
Redirections for 'https://public.apps.ubuntu.com/anon/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap': https://068ed04f23.site.internapcdn.net/download-snap/99T7MUlRhtI3U0QFgl5mXXESAiSwt776_1689.snap?t=2017-05-25T22:36:28Z&h=81eb231649cba42085e9a5ab0ded3b756f0c9881
headers = {}
if os.path.exists(download_path):
headers['Range'] = 'StartPos: %d'.format(
os.path.getsize(download_path))

This comment has been minimized.

Copy link
@cprov

cprov May 26, 2017

Contributor

Internap has Accept-Ranges: bytes, so we should be using Range: bytes=<size>-'.

See the full workflow in action with curl in https://pastebin.canonical.com/189327/

@kyrofa
Copy link
Member

left a comment

One nit and a few questions. From the end-user's perspective, this will look just like it does today (show an error and try again), but it'll resume where it left off. Right? With the resume logic, do we even need to consider that a failure? Can we simply continue trying while showing the same progress bar?

"""This is a facility to download a request with nice progress bars."""

# Doing len(request_stream.content) may defeat the purpose of a
# progress bar
total_length = 0
if not request_stream.headers.get('Content-Encoding', ''):
total_length = int(request_stream.headers.get('Content-Length', '0'))
# Content-Lenght in the case of resuming will be

This comment has been minimized.

Copy link
@kyrofa

kyrofa May 26, 2017

Member

s/Lenght/Length/

# Content-Length - total_read so we add back up to have the feel of
# resuming
if os.path.exists(destination):
total_length += total_read

This comment has been minimized.

Copy link
@kyrofa

kyrofa May 26, 2017

Member

Nice, this is to make the progress bar start partially completed, essentially?

This comment has been minimized.

Copy link
@sergiusens

sergiusens May 26, 2017

Author Collaborator

yes

probe_url = requests.head(download_url)
if probe_url.is_redirect:
download_url = probe_url.headers['Location']
resume_possible = True

This comment has been minimized.

Copy link
@kyrofa

kyrofa May 26, 2017

Member

Can you explain this? Why is a resume possible only if the download URL is a redirect?

This comment has been minimized.

Copy link
@sergiusens

sergiusens May 26, 2017

Author Collaborator

it is the only place I know where Accept-Ranges: bytes works.

headers = {}
if resume_possible and os.path.exists(download_path):
total_read = os.path.getsize(download_path)
headers['Range'] = 'bytes={}-'.format(total_read)

This comment has been minimized.

Copy link
@kyrofa

kyrofa May 26, 2017

Member

The equals sign doesn't appear to be part of the format (at least according to this). To what spec are you referring?

This comment has been minimized.

Copy link
@sergiusens

sergiusens May 26, 2017

Author Collaborator

look at @cprov's comment above.

This comment has been minimized.

Copy link
@cprov

cprov May 26, 2017

Contributor

Right, the specification does not require it, space should be fine, perhaps it's a specialty of internap :-/

Anyway, as I've noted in the previous comment, this is how it's currently working, but it's worth noting that it contains internap hacks. Other than that 👍

@sergiusens

This comment has been minimized.

Copy link
Collaborator Author

commented May 28, 2017

@kyrofa and @cprov one last final thing to not do the wrong thing in the future if you don't mind taking a look

@elopio elopio changed the title storeapi: log download retries storeapi: resume snap downloads May 28, 2017

@elopio

elopio approved these changes May 28, 2017

Copy link
Contributor

left a comment

looks good and the tests are green. Lets see if it causes less issues.

@sergiusens sergiusens added this to the 2.31 milestone May 28, 2017

@sergiusens sergiusens merged commit 117c328 into snapcore:master May 28, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@sergiusens sergiusens deleted the sergiusens:download_retry_debug_logging branch May 28, 2017

@sergiusens sergiusens modified the milestones: 2.31, 2.30.1 Jun 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.