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

stream webdav downloads using http client #18653

Merged
merged 3 commits into from
Sep 11, 2015
Merged

Conversation

icewind1991
Copy link
Contributor

Extends the http client to allow streamed responses and uses that for dav downloads.

Replaces #10620

cc @PVince81 @LukasReschke @MorrisJobke

@ghost
Copy link

ghost commented Aug 29, 2015

🚀 Test PASSed.🚀
chuck

@RobinMcCorkell
Copy link
Member

Why use Guzzle instead of the Sabre DAV Client that is already used in that class? It supports streams as well, with nearly the same interface:

$request = new \Sabre\HTTP\Request('GET', $this->createBaseUri() . $this->encodePath($path));

$response = $this->client->send($request);

return $response->getBodyAsStream();

Also, you missed the cURL stuff in uploadFile().

@MorrisJobke MorrisJobke added this to the 8.2-current milestone Aug 29, 2015
@icewind1991
Copy link
Contributor Author

From what I can tell that api doesn't actually stream the result, just returns the string result as a stream

cc @evert

@evert
Copy link

evert commented Aug 30, 2015

This is true.. It's been a feature request for a while: sabre-io/http#15

@RobinMcCorkell
Copy link
Member

@evert As @DeepDiver1975 sometimes says... FIXIT! 😆

@evert
Copy link

evert commented Aug 30, 2015

I don't know how!

@icewind1991
Copy link
Contributor Author

@Xenopathic @PVince81 @DeepDiver1975 can this be merged?

* @param GuzzleResponse $response
* @param bool $stream
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$isStream ? Is it a boolean or can it be an actual stream ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A boolean, to tell the Response class if it should treat the response as a stream

@PVince81
Copy link
Contributor

PVince81 commented Sep 1, 2015

Seems to work with federated shares 👍

Would be good to find out whether it works with valid SSL certs though. @LukasReschke what do you think ?

@RobinMcCorkell
Copy link
Member

Still missing the other cURL stuff in uploadFile()

@RobinMcCorkell
Copy link
Member

👍

@scrutinizer-notifier
Copy link

A new inspection was created.

@RobinMcCorkell
Copy link
Member

👍

@icewind1991
Copy link
Contributor Author

@PVince81 can you re-review with the latest changes

@icewind1991
Copy link
Contributor Author

@DeepDiver1975 @MorrisJobke can you review this?

@LukasReschke
Copy link
Member

👍

LukasReschke added a commit that referenced this pull request Sep 11, 2015
stream webdav downloads using http client
@LukasReschke LukasReschke merged commit 1924dd3 into master Sep 11, 2015
@LukasReschke LukasReschke deleted the dav-stream-guzzle branch September 11, 2015 15:10
@lock lock bot locked as resolved and limited conversation to collaborators Aug 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants