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

session: api to resume canceled downloads from previous session #8061

Merged
merged 5 commits into from Dec 9, 2016

Conversation

Projects
None yet
4 participants
@deepak1556
Member

deepak1556 commented Nov 23, 2016

  • Add spec

Fixes #7720

/cc @idododu

Show outdated Hide outdated docs/api/download-item.md
Show outdated Hide outdated docs/api/download-item.md
Show outdated Hide outdated docs/api/session.md
Show outdated Hide outdated docs/api/session.md
@deepak1556

This comment has been minimized.

Show comment
Hide comment
@deepak1556

deepak1556 Nov 24, 2016

Member

@zeke Fixed, thanks!

Member

deepak1556 commented Nov 24, 2016

@zeke Fixed, thanks!

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Dec 6, 2016

Contributor

@deepak1556 this is really cool, left a few minor comments 👍

Contributor

kevinsawicki commented Dec 6, 2016

@deepak1556 this is really cool, left a few minor comments 👍

@kevinsawicki kevinsawicki self-assigned this Dec 6, 2016

@deepak1556

This comment has been minimized.

Show comment
Hide comment
@deepak1556

deepak1556 Dec 7, 2016

Member

Have addressed the comments, thanks!

Member

deepak1556 commented Dec 7, 2016

Have addressed the comments, thanks!

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Dec 9, 2016

Contributor

Thanks for this @deepak1556, rebased it and made one small spec tweak, 🚢 🚢 🚢

Contributor

kevinsawicki commented Dec 9, 2016

Thanks for this @deepak1556, rebased it and made one small spec tweak, 🚢 🚢 🚢

@kevinsawicki kevinsawicki merged commit 952e3ba into electron:master Dec 9, 2016

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@thejhnz

This comment has been minimized.

Show comment
Hide comment
@thejhnz

thejhnz Mar 6, 2017

I am currently writing support for closing the application and for the download to finish when they re-open.

For some reason when resuming a "createInterruptedDownload" call it just doesn't work. canResume() returns false all the time. According to the API, the createInterruptedDownload function is meant to be able to be resumed?

I may be confused with what 'Resume' is, is it downloading from the start? I assumed that if you are downloading a 4gb file, download 1gb, application closes, do the createInterruptedDownload with the 1GB offset and it would continue. This is not the case at the moment.

If you guys could check it out to see if this is working correctly that would be great.

thejhnz commented Mar 6, 2017

I am currently writing support for closing the application and for the download to finish when they re-open.

For some reason when resuming a "createInterruptedDownload" call it just doesn't work. canResume() returns false all the time. According to the API, the createInterruptedDownload function is meant to be able to be resumed?

I may be confused with what 'Resume' is, is it downloading from the start? I assumed that if you are downloading a 4gb file, download 1gb, application closes, do the createInterruptedDownload with the 1GB offset and it would continue. This is not the case at the moment.

If you guys could check it out to see if this is working correctly that would be great.

@deepak1556

This comment has been minimized.

Show comment
Hide comment
@deepak1556

deepak1556 Mar 6, 2017

Member

canResume() returns false all the time.

This is something that needs to be fixed. You can check the state of the download item instead.

I assumed that if you are downloading a 4gb file, download 1gb, application closes, do the createInterruptedDownload with the 1GB offset and it would continue. This is not the case at the moment.

Yes it should resume if you have provided the right parameters, have you tried resuming the download ?

Member

deepak1556 commented Mar 6, 2017

canResume() returns false all the time.

This is something that needs to be fixed. You can check the state of the download item instead.

I assumed that if you are downloading a 4gb file, download 1gb, application closes, do the createInterruptedDownload with the 1GB offset and it would continue. This is not the case at the moment.

Yes it should resume if you have provided the right parameters, have you tried resuming the download ?

@thejhnz

This comment has been minimized.

Show comment
Hide comment
@thejhnz

thejhnz Mar 6, 2017

Thanks for your reply.

I think am passing the correct parameters as it does send it to 'will-download'. I log the received bytes and it has to offset but once I resume it just changes the received bytes back to 0 and starts fresh. It does however create a new file, such as 'file (1).rar' and does not continue onto the other file 'file.rar'.

thejhnz commented Mar 6, 2017

Thanks for your reply.

I think am passing the correct parameters as it does send it to 'will-download'. I log the received bytes and it has to offset but once I resume it just changes the received bytes back to 0 and starts fresh. It does however create a new file, such as 'file (1).rar' and does not continue onto the other file 'file.rar'.

@deepak1556

This comment has been minimized.

Show comment
Hide comment
@deepak1556

deepak1556 Mar 7, 2017

Member

@thejhnz does the server responding file.rar support range requests ?

Member

deepak1556 commented Mar 7, 2017

@thejhnz does the server responding file.rar support range requests ?

@thejhnz

This comment has been minimized.

Show comment
Hide comment
@thejhnz

thejhnz Mar 7, 2017

Yup

HTTP/1.1 206 Partial Content
Date: Tue, 07 Mar 2017 07:50:39 GMT
Content-Type: application/x-rar-compressed
Content-Length: 61
Connection: close
Set-Cookie: __cfduid=d76d46be1b53f6d814ab1ef7e2d10df401488873039; expires=Wed, 07-Mar-18 07:50:39 GMT; path=/; domain=.xeragame.com; HttpOnly
Last-Modified: Wed, 04 Jan 2017 00:30:50 GMT
Accept-Ranges: bytes
Content-Range: bytes 60-120/4325188445
Server: cloudflare-nginx
CF-RAY: 33bbf7139450190e-AKL

thejhnz commented Mar 7, 2017

Yup

HTTP/1.1 206 Partial Content
Date: Tue, 07 Mar 2017 07:50:39 GMT
Content-Type: application/x-rar-compressed
Content-Length: 61
Connection: close
Set-Cookie: __cfduid=d76d46be1b53f6d814ab1ef7e2d10df401488873039; expires=Wed, 07-Mar-18 07:50:39 GMT; path=/; domain=.xeragame.com; HttpOnly
Last-Modified: Wed, 04 Jan 2017 00:30:50 GMT
Accept-Ranges: bytes
Content-Range: bytes 60-120/4325188445
Server: cloudflare-nginx
CF-RAY: 33bbf7139450190e-AKL
@deepak1556

This comment has been minimized.

Show comment
Hide comment
@deepak1556

deepak1556 Mar 7, 2017

Member

Thanks! can you provide a minimal example to reproduce the issue ? It will help to debug the problem.

Member

deepak1556 commented Mar 7, 2017

Thanks! can you provide a minimal example to reproduce the issue ? It will help to debug the problem.

@thejhnz

This comment has been minimized.

Show comment
Hide comment
@thejhnz

thejhnz Mar 25, 2017

I figured out that it needs the lastModified tag. Adding appears to have fixed the issue. It might be best to throw an error if this is not specified.

thejhnz commented Mar 25, 2017

I figured out that it needs the lastModified tag. Adding appears to have fixed the issue. It might be best to throw an error if this is not specified.

@deepak1556

This comment has been minimized.

Show comment
Hide comment
@deepak1556

deepak1556 Mar 25, 2017

Member

@thejhnz lastModified was not specified as optional in the docs, but I guess its better to throw error when not specified. Can you open an issue ? Also if you feel like submitting a PR , go ahead.

Member

deepak1556 commented Mar 25, 2017

@thejhnz lastModified was not specified as optional in the docs, but I guess its better to throw error when not specified. Can you open an issue ? Also if you feel like submitting a PR , go ahead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment