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

create nested exiter-contextmanagers #7597

Closed
ity opened this issue Apr 19, 2019 · 0 comments · Fixed by #9722
Closed

create nested exiter-contextmanagers #7597

ity opened this issue Apr 19, 2019 · 0 comments · Fixed by #9722

Comments

@ity
Copy link
Contributor

ity commented Apr 19, 2019 •
edited
Loading

Problem

Currently, when DaemonPantsRunner calls LocalPantsRunner it passes in a DaemonExiter in create (

pants/src/python/pants/bin/daemon_pants_runner.py

Line 323 in c443c45

runner = LocalPantsRunner.create(
) which is implicitly reset/overwritten by LocalExiter within set_start_time()(

pants/src/python/pants/bin/local_pants_runner.py

Line 231 in f555ff2

self._exiter = LocalExiter(self._run_tracker, self._repro, exiter=self._exiter)
) when its invoked(

pants/src/python/pants/bin/daemon_pants_runner.py

Line 331 in c443c45

runner.set_start_time(self._maybe_get_client_start_time_from_env(self._env))
).

The issue is when we try to call self._exiter.exit() in any of the other method paths such as

pants/src/python/pants/bin/daemon_pants_runner.py

Line 334 in c443c45

self._exiter.exit_and_fail('Interrupted by user.\n')
the self._exiter instance has already been replaced. (Workarounds have been used to reset the exiter but this isnt scalable).

Solution

Create nested exiter-contextmanagers in the DaemonPantsRunner and LocalPantsRunner interaction

  1. the outer one corresponding to the DaemonExiter and
  2. an inner one corresponding to the LocalExiter which will be used and then reset at the end of the run.

Result

happy exiting!
@ity ity changed the title create nested exiter-contextmanagers within LocalPantsRunner#_run() create nested exiter-contextmanagers in the DaemonPantsRunner and LocalPantsRunner interaction Apr 19, 2019
@ity ity changed the title create nested exiter-contextmanagers in the DaemonPantsRunner and LocalPantsRunner interaction create nested exiter-contextmanagers Apr 19, 2019
@ity ity mentioned this issue Apr 23, 2019
Closed
@blorente blorente mentioned this issue Jul 9, 2019
Closed
@stuhood stuhood mentioned this issue May 10, 2020
Merged
stuhood pushed a commit that referenced this issue May 13, 2020
Port the pantsd nailgun server to rust (#9722)
9506fbf 
### Problem

The setup and teardown of each request made to the nailgun server in `pantsd` had become quite complicated over time... and consequently, slower than it needed to be.

### Solution

Port `pantsd`'s nailgun server to rust using the `nails` crate. Additionally, remove the `Exiter` class, which had accumulated excess responsibilities that can instead be handled by returning `ExitCode` values. Finally, fix a few broken windows including: double logging to pantsd, double help output, closed file errors on pantsd shutdown, and redundant setup codepaths.

### Result

There is less code to maintain, and runs of `./pants --enable-pantsd help` take `~1.7s`, of which `~400ms` are spent in the server. Fixes #9448, fixes #8243, fixes #8206, fixes #8127, fixes #7653, fixes #7613, fixes #7597.
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

Successfully merging a pull request may close this issue.

Port the pantsd nailgun server to rust stuhood/pants
1 participant
@ity