Remove unnecessary process wait on MacOS (issue #1523) #1526
Conversation
- remove process.communicate() in MacOS causing wait for subprocess even when the subprocess results were not used.
cedel1
force-pushed
the
check_lagging_issue_1523
branch
from
8241f4b
to
57e6f1f
Compare
|
Hmm, I am surprised by the unrelated codecov check failure (especially since that part wasn't changed for quite some time, it fails on master and tests in previous PRs passed. Version changes? |
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1526 +/- ##
==========================================
- Coverage 97.28% 94.30% -2.99%
==========================================
Files 67 113 +46
Lines 4235 7688 +3453
==========================================
+ Hits 4120 7250 +3130
- Misses 115 438 +323 ☔ View full report in Codecov by Sentry. |
| @property | ||
| def command(self): | ||
| cmd = ' '.join(shlex.quote(arg) for arg in ['http', *self.args]) | ||
| # pytest-httpbin to real httpbin. | ||
| return re.sub(r'127\.0\.0\.1:\d+', 'httpbin.org', cmd) | ||
|
|
There was a problem hiding this comment.
pycodestyle (flake) complains that this redefines the command property a few lines above.
While it would be possible to correct that by removing the property from the list (and possibly adding setter and delete version of the @property to be able to change its value and/or delete the value), the only usage I found was a commented-out line a few lines below, so it made sense to me to just remove it completely, since it is basically unused code.
| process = _start_process(args, env=process_context) | ||
| # Unlike windows, since we already completed the fork procedure | ||
| # we can simply join the process and wait for it. | ||
| process.communicate() |
There was a problem hiding this comment.
Waited for the process and could send some data to it or return its return code - but nothing of the sort is being done, basically blocking until the process finishes. But since the purpose of the process is to be daemonized (or is possibly daemonized already), it doesn't seem to make sense.
cedel1
changed the title
Remove
process.communicate()in MacOS causing wait for subprocess even when the subprocess results were not used.