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

Add App::terminate() stream support #2028

Merged
merged 25 commits into from
Apr 8, 2023

Conversation

DarkSide666
Copy link
Member

No description provided.

@DarkSide666 DarkSide666 marked this pull request as draft April 4, 2023 09:53
@@ -68,4 +70,22 @@ public function testUnexpectedOutputLateError(): void
ob_end_clean();
}
}

/* throws headers already sent exception so not sure how to write this test for stream output
public function testStreamResponse(): void
Copy link
Member

Choose a reason for hiding this comment

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

this test should test sending large (10GB+) data with known pattern to test the response is not materialized somewhere

Copy link
Member Author

Choose a reason for hiding this comment

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

To be honest I have no idea how to write that. Will have to ask ChatGPT :)

Copy link
Member

@mvorisek mvorisek Apr 8, 2023

Choose a reason for hiding this comment

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

implemented and tested with 6GB

the main bottleneck seems to be in Guzzle HTTP client - guzzle/guzzle#3115

until improved, let's test the streaming support with 50 MB only

src/App.php Outdated Show resolved Hide resolved
@mvorisek mvorisek changed the title Add Stream argument support to terminate() method Add App::terminate() stream support Apr 4, 2023
@mvorisek mvorisek removed their assignment Apr 5, 2023
@mvorisek mvorisek force-pushed the feature/support-stream-in-terminate branch from 298887b to fa76c11 Compare April 7, 2023 00:21
@mvorisek mvorisek force-pushed the feature/support-stream-in-terminate branch from e0493a8 to 6801f94 Compare April 7, 2023 22:53
@mvorisek mvorisek force-pushed the feature/support-stream-in-terminate branch from 29a2bc2 to 65fbd8b Compare April 8, 2023 10:30
@mvorisek mvorisek marked this pull request as ready for review April 8, 2023 10:33
@mvorisek mvorisek merged commit d238f44 into develop Apr 8, 2023
@mvorisek mvorisek deleted the feature/support-stream-in-terminate branch April 8, 2023 11:17
@DarkSide666 DarkSide666 assigned mvorisek and unassigned mvorisek Apr 11, 2023
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.

3 participants