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

Feat uds for log file #1120

Closed
wants to merge 10 commits into from
Closed

Feat uds for log file #1120

wants to merge 10 commits into from

Conversation

nguyenhoangthuan99
Copy link
Contributor

No description provided.

@nguyenhoangthuan99 nguyenhoangthuan99 marked this pull request as ready for review September 6, 2024 04:04
@dan-homebrew
Copy link
Contributor

@nguyenhoangthuan99 I am not merging this yet, as I am not sure if this approach is correct:

  • It seems that we are doing this to bypass a sensible Windows protection (e.g. single process lock on file)
  • This seems fragile;

I have commented in the original Issue:

#1153

  • Should we consider separate logfiles for CLI and server, as 2 separate processes?
  • This could be a simpler change

Additionally, when you re-submit the PR, please make sure you link the issue https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue

@nguyenhoangthuan99
Copy link
Contributor Author

I think we should close this PR, because we no longer need UDS. I'll update the new logger in another PR

@dan-homebrew
Copy link
Contributor

@nguyenhoangthuan99 I typically have submitters close their own PR - can you do so when ready?

@nguyenhoangthuan99
Copy link
Contributor Author

I'll close this PR.

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.

bug: Multiprocess cannot write log to the same file
2 participants