Skip to content

Task refactoring #47

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

Merged
merged 24 commits into from
Jun 24, 2019
Merged

Task refactoring #47

merged 24 commits into from
Jun 24, 2019

Conversation

artemredkin
Copy link
Collaborator

Instead of having promise and a future inside Handler and Task, I propose to hide the promise inside the Task and drop future from it

Motivation:
Users may want to optimize memory usage when executing request with big
bodies, same as streaming download

Modifications:
Added ChunkProvider typealias in HTTPHandler.swift
Added new Body enum case - stream in HTTPHandler.swift
Extracted body processing in HTTPHandler.swift to a separate method
Added .stream enum processing in HTTPHandler.swift
Added upload streaming test to SwiftNIOHTTPTests.swift

Result:
HTTPClient library now provides methods to stream Request body
@@ -272,8 +276,16 @@ public extension HTTPClient {
}
}

public func succeed(_ value: Response) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be public? Should a user of the library be able to succeed/fail the task themselves?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point, this shouldn't be public, thanks!

Copy link
Member

@tanner0101 tanner0101 left a comment

Choose a reason for hiding this comment

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

+1 to storing the promise.

@tanner0101 tanner0101 added the kind/enhancement Improvements to existing feature. label Jun 20, 2019
@artemredkin artemredkin merged commit 34e3403 into master Jun 24, 2019
@artemredkin artemredkin deleted the task_refactoring branch June 24, 2019 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements to existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants