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

suggestion: rework std/log to use Streams API #3907

Closed
iuioiua opened this issue Dec 7, 2023 · 3 comments
Closed

suggestion: rework std/log to use Streams API #3907

iuioiua opened this issue Dec 7, 2023 · 3 comments

Comments

@iuioiua
Copy link
Contributor

iuioiua commented Dec 7, 2023

std/log is the last of the sub-modules that uses the deprecated Writer (i.e. Deno.Writer) interface. It should use WritableStream instead.

Related:

@syhol
Copy link
Contributor

syhol commented Dec 15, 2023

I'm having a go at this.

It looks like moving to the Streams API causes some of the handler methods to become async, this might introduce complexities around what happens if a log comes in mid flush as operations are no longer atomic, especially so for the RotatingFileHandler.

Would you mind if I submit a WIP PR to see if I'm on the right track? I might need to implement a queue to buffer the buffer.

@iuioiua
Copy link
Contributor Author

iuioiua commented Dec 17, 2023

That's great to hear! I've looked at your PR, and it seems like you're on the right track for now. Please let us know if you have any questions.

@iuioiua
Copy link
Contributor Author

iuioiua commented Dec 28, 2023

An alternative was provided in #4021 that used Deno.FsFile.writeSync() and removed the dependency on Deno.Writer. Thank you for solving this, @syhol!

@iuioiua iuioiua closed this as completed Dec 28, 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

No branches or pull requests

2 participants