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

Remove head request #3809

Conversation

kulmann
Copy link
Member

@kulmann kulmann commented Jul 22, 2020

Same as #3797, but without the HEAD request. If a download fails, it just fails. No need to determine beforehand if it could fail.

@kulmann kulmann requested a review from IljaN July 22, 2020 15:14
@update-docs
Copy link

update-docs bot commented Jul 22, 2020

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@kulmann
Copy link
Member Author

kulmann commented Jul 22, 2020

@IljaN check out this PR and run yarn build in phoenix. Restarting ocis should not be required. If the download works in the browser, everything is fine.

@IljaN
Copy link
Member

IljaN commented Jul 22, 2020

Works for me:
signed_download

@kulmann
Copy link
Member Author

kulmann commented Jul 22, 2020

@IljaN the actual download is not visible in the network tab - only as file in your download list. did that appear?

@IljaN
Copy link
Member

IljaN commented Jul 22, 2020

Yeah sorry I see now that the image is cropped wrongly... Nothing more visible in the Network tab. But browser Download-File picker appeared and I could download file.

@kulmann
Copy link
Member Author

kulmann commented Jul 22, 2020

Screenshot 2020-07-22 at 19 01 06

successfully downloaded a 6GB random data file (on localhost) with this PR.

@kulmann
Copy link
Member Author

kulmann commented Jul 22, 2020

failure handling needs to be fixed/decided before we can merge this.

With the browser api this is what happens on failures:
Screenshot 2020-07-22 at 21 16 45

So there is no error dialog triggered in phoenix. Which could be ok. Need to decide it.

@kulmann kulmann mentioned this pull request Jul 22, 2020
63 tasks
@kulmann
Copy link
Member Author

kulmann commented Jul 23, 2020

Closing this, since we decided to keep (and fix) the HEAD request for now. Otherwise we'd need to change all download related tests.

@kulmann kulmann closed this Jul 23, 2020
@pascalwengerter pascalwengerter deleted the signed-urls-download-without-head-req branch June 2, 2021 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants