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

ProcessHandler: Make timeout of stream_select() configurable #1916

Merged
merged 3 commits into from
Oct 23, 2024

Conversation

ericemmrich
Copy link
Contributor

In our application, any request has always been blocked for 1 second and made it significantly slower. Rather than needing to overwrite almost the entire class with a custom implementation, it would be nice to have this timeout configurable.

In our application, any request has always been blocked for 1 second and made it significantly slower. Rather than needing to overwrite almost the entire class with a custom implementation, it would be nice to have this timeout configurable.
@ericemmrich
Copy link
Contributor Author

@Seldaek What would be required to move this forward? :)

@Seldaek
Copy link
Owner

Seldaek commented Oct 23, 2024

Do you use ProcessHandler in prod? What for? Just out of curiosity sorry..

Anyway the PR seems fine so I'll merge but I'll need some more free time before I can triage other stuff and tag a release.

@ericemmrich
Copy link
Contributor Author

Do you use ProcessHandler in prod? What for? Just out of curiosity sorry..

Anyway the PR seems fine so I'll merge but I'll need some more free time before I can triage other stuff and tag a release.

No worries, thanks for picking this up! :)

We use the ProcessHandler for redirecting some logs to the stderr of process ID 1, so that they are displayed properly in the container logs:

            'handler' => \Monolog\Handler\ProcessHandler::class,
            'with' => [
                'command' => 'cat >> /proc/1/fd/2',
            ],

Copy link
Owner

@Seldaek Seldaek left a comment

Choose a reason for hiding this comment

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

Just a suggestion to allow passing a float as timeout so you can set microseconds too, because just having 0 or 1 is not allowing for a lot of precision.

src/Monolog/Handler/ProcessHandler.php Outdated Show resolved Hide resolved
src/Monolog/Handler/ProcessHandler.php Outdated Show resolved Hide resolved
src/Monolog/Handler/ProcessHandler.php Outdated Show resolved Hide resolved
src/Monolog/Handler/ProcessHandler.php Outdated Show resolved Hide resolved
@Seldaek Seldaek merged commit 2497a89 into Seldaek:main Oct 23, 2024
16 of 17 checks passed
@Seldaek
Copy link
Owner

Seldaek commented Oct 23, 2024

Alright, thanks

@Seldaek Seldaek added this to the 3.x milestone Oct 23, 2024
@ericemmrich ericemmrich deleted the patch-1 branch October 23, 2024 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants