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.

Sign up for GitHub

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

Jump to bottom

@ensure_daemon does not terminate with Python 3.7 #7160

Closed
Eric-Arellano opened this issue Jan 25, 2019 · 2 comments
Closed

@ensure_daemon does not terminate with Python 3.7 #7160

Eric-Arellano opened this issue Jan 25, 2019 · 2 comments

Comments

@Eric-Arellano
Copy link
Contributor

Eric-Arellano commented Jan 25, 2019 •
edited
Loading

While #7130 fixes the @ensure_daemon for Python 3.6 and thus CI, with Python 3.7 the decorator will still lead to a timeout.

For example, run ./pants3 clean-all test tests/python/pants_test/engine/legacy:owners_integration -- -k test_owner_of_success.

Not only does the process time out, but it never terminates. Apply this diff and rerun the above command:

diff --git a/src/python/pants/pantsd/process_manager.py b/src/python/pants/pantsd/process_manager.py
index e42cbfa6e..e01629420 100644
--- a/src/python/pants/pantsd/process_manager.py
+++ b/src/python/pants/pantsd/process_manager.py
@@ -131,8 +131,8 @@ class ProcessMetadataManager(object):
         return True

       now = time.time()
-      if now > deadline:
-        raise cls.Timeout('exceeded timeout of {} seconds while waiting for {}'.format(timeout, action_msg))
+      # if now > deadline:
+      #   raise cls.Timeout('exceeded timeout of {} seconds while waiting for {}'.format(timeout, action_msg))

       if now > info_deadline:
         logger.info('waiting for {}...'.format(action_msg))

Note we also had this issue when using Python 3.6 originally with #7130, until adding the commit f94550e.

--

(Tip to get Python 3.7. Use pyenv install 3.7.2 && pyenv global 3.7.2, along with rm -rf ~/.cache/pants/python_cache/interpreters to remove a bad symlink.)
@Eric-Arellano Eric-Arellano mentioned this issue Jan 25, 2019
Merged
@OniOni
Copy link
Contributor

OniOni commented Jan 25, 2019

I'm going to try and get this running on Linux with 3.7, so we can rule out osx as a culprit.

@Eric-Arellano Eric-Arellano mentioned this issue Jan 25, 2019
Closed
Eric-Arellano added a commit that referenced this issue Jan 26, 2019
@Eric-Arellano
Properly handle unicode and byte streams with pantsd for Python 3 (#7130
c5d4ea1 
)

## Problem
Any test with `@ensure_daemon` fails when ran with `./pants3`. This is due to unicode issues.

For example, `./pants3 test tests/python/pants_test/base:exiter_integration` will fail with:

```python
Exception caught: (builtins.TypeError)
  File "/home/eric/pants/src/python/pants/bin/pants_loader.py", line 89, in <module>
    main()
  File "/home/eric/pants/src/python/pants/bin/pants_loader.py", line 85, in main
    PantsLoader.run()
  File "/home/eric/pants/src/python/pants/bin/pants_loader.py", line 81, in run
    cls.load_and_execute(entrypoint)
  File "/home/eric/pants/src/python/pants/bin/pants_loader.py", line 74, in load_and_execute
    entrypoint_main()
  File "/home/eric/pants/src/python/pants/bin/pants_exe.py", line 39, in main
    PantsRunner(exiter, start_time=start_time).run()
  File "/home/eric/pants/src/python/pants/bin/pants_runner.py", line 48, in run
    return RemotePantsRunner(self._exiter, self._args, self._env, options_bootstrapper).run()
  File "/home/eric/pants/src/python/pants/bin/remote_pants_runner.py", line 190, in run
    self._run_pants_with_retry(pantsd_handle)
  File "/home/eric/pants/src/python/pants/bin/remote_pants_runner.py", line 114, in _run_pants_with_retry
    return self._connect_and_execute(pantsd_handle.port)
  File "/home/eric/pants/src/python/pants/bin/remote_pants_runner.py", line 155, in _connect_and_execute
    result = client.execute(self.PANTS_COMMAND, *self._args, **modified_env)
  File "/home/eric/pants/src/python/pants/java/nailgun_client.py", line 269, in execute
    return self._session.execute(cwd, main_class, *args, **environment)
  File "/home/eric/pants/src/python/pants/java/nailgun_client.py", line 109, in execute
    return self._process_session()
  File "/home/eric/pants/src/python/pants/java/nailgun_client.py", line 80, in _process_session
    self._write_flush(self._stdout, payload)
  File "/home/eric/pants/src/python/pants/java/nailgun_client.py", line 63, in _write_flush
    fd.write(payload)

Exception message: write() argument must be str, not bytes
```

## Solution
Use `sys.std{out,err}.buffer` with Py3. 

We reaffirmed in #7073 a prior decision that the `Exiter` related code should be using a bytes interface. However, we did not fix the pantsd related code because it was causing regressions. We now fix these usages.

Note that in `pants_daemon.py`, we override `sys.stdout` to our own custom `_LoggerStream` object. To ensure Python 3 support, we add a `buffer` property.

## Caveats
### Exiter integration still fails on macOS
`tests/python/pants_test/base:exiter_integration` will fail on macOS still, due to an upstream non fork safe osx lib (see https://bugs.python.org/issue28342). But this bug also affects Python 2.

### Python 3.7 daemon never executes
See #7160.
@Eric-Arellano Eric-Arellano mentioned this issue Jan 27, 2019
Merged
@Eric-Arellano
Copy link
Contributor Author

Closed by #7175. Thank you Mathieu!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@OniOni @Eric-Arellano