Skip to content

Add console as config in ioRuntimeConfig, pass it to CPUStarvation #3496

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

Conversation

manuelcueto
Copy link

@manuelcueto manuelcueto commented Mar 15, 2023

Addresses #3495
Change CpuStarvation to take a function obtaining the metrics when there's a clock drift, to give the ability to the user to do as they want with the information. Defaults to the previous mechanism of logging into Sys.err

* Defines what to do when CpuStarvationCheck is triggered. Defaults to log a warning to
* System.err.
*/
protected def onCpuStarvationWarn: CpuStarvationWarningMetrics => IO[Unit] =
Copy link
Member

Choose a reason for hiding this comment

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

Actually one quick nit: shouldn't this be a method, rather than a function?

Suggested change
protected def onCpuStarvationWarn: CpuStarvationWarningMetrics => IO[Unit] =
protected def onCpuStarvationWarn(metrics: CpuStarvationWarningMetrics): IO[Unit] =

It means we can't define it in a point-free fashion, but the users are much more likely to define this in a non-point-free fashion, similar to reportFailure.

Copy link
Author

Choose a reason for hiding this comment

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

Will look into this tomorrow morning. 👍 Thanks for the review

@djspiewak
Copy link
Member

Thank you for taking this on!

@djspiewak djspiewak merged commit 1e92538 into typelevel:series/3.x May 3, 2023
@manuelcueto manuelcueto deleted the 3495-configurable-starvation-warning-console branch May 4, 2023 08:24
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