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

Pass log-file and timeout args to daemon as proper args #15227

Merged
merged 3 commits into from
May 16, 2023

Conversation

svalentin
Copy link
Collaborator

@svalentin svalentin commented May 12, 2023

Currently, when starting the mypy daemon, we're passing the timeout and log_file as part of the pickled options. This can cause problems if the daemon has issues with the pickling. In this case we won't be able to log what the problem was.
Let's pass the log_file and timeout as first class args. This also has the advantage that we can now start the daemon in the foreground with these args from the command line in a human writeable way.

Currently, when starting the mypy daemon, we're passing the timeout and
log_file as part of the pickled options. This can cause problems if the
daemon has issues with the pickling.
Lets pass the log_file and timeout as first class args.
This also has the advantage that we can now start the daemon in the
foreground with these args from the command line in a human writeable
way.
@svalentin svalentin requested a review from JukkaL May 12, 2023 00:26
pre-commit-ci bot and others added 2 commits May 12, 2023 00:27
@github-actions
Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@svalentin svalentin merged commit ce2c918 into python:master May 16, 2023
@svalentin svalentin deleted the dmypy-flags branch May 16, 2023 17:50
@CoolCat467
Copy link

What about Linux or Mac OS? These changes only apply for

sys.platform == "win32"

and now on Linux I have no log files anymore

@svalentin
Copy link
Collaborator Author

svalentin commented Jul 4, 2023

What about Linux or Mac OS? These changes only apply for

sys.platform == "win32"

and now on Linux I have no log files anymore

Linux shouldn't use the pickle route. I don't think it's this PR that caused issued, but I'll take a look

@svalentin
Copy link
Collaborator Author

What about Linux or Mac OS? These changes only apply for

sys.platform == "win32"

and now on Linux I have no log files anymore

I retested this in every scenario under Linux. It works fine for me.

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.

2 participants