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

JSON workers should delimit work requests with newlines #14218

Closed
wolfd opened this issue Nov 2, 2021 · 4 comments
Closed

JSON workers should delimit work requests with newlines #14218

wolfd opened this issue Nov 2, 2021 · 4 comments

Comments

@wolfd
Copy link
Contributor

wolfd commented Nov 2, 2021

Description of the problem / feature request:

JSON workers are difficult to write because work requests are not newline delimited. This issue was brought up in #13599, and the discussion generally indicated that this is a usability bug.

The proposal is to add a newline between requests according to ndjson.

Feature requests: what underlying problem are you trying to solve with this feature?

Right now an implementation of a JSON worker work request loop using python looks like this:

# This is a pretty cumbersome way of getting work requests.
# It waits for something to be available on stdin, and then tries parsing
# whatever text is available as JSON. It keeps accumulating more text until a
# full work request is available.
def wait_for_stdin_json():
    stdin = open("/dev/stdin", "rb")
    os.set_blocking(stdin.fileno(), False)
    stream_bytes = b""
    while True:
        _, _, _ = select.select([stdin.fileno()], [], [])
        maybe_bytes = stdin.read()
        if maybe_bytes is None:
            continue
        stream_bytes += maybe_bytes
        try:
            work_request = json.loads(stream_bytes)
        except json.decoder.JSONDecodeError:
            # stream couldn't decode, wait until next select
            sys.stderr.write(f"failed to decode: {stream_bytes.decode('utf-8')}\n")
            sys.stderr.flush()
            continue

        yield work_request
        stream_bytes = b""  # reset

With newline-delimited JSON it would look like this:

# Simplified json loop with newline-delimited json.
def wait_for_stdin_json():
    while True:
        yield json.loads(sys.stdin.readline())

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Current version

Proposed version

@meisterT
Copy link
Member

meisterT commented Nov 3, 2021

I agree, see #13599 (comment)

@meisterT
Copy link
Member

meisterT commented Nov 3, 2021

cc @larsrc-google

@brentleyjones
Copy link
Contributor

@wolfd Do you want to make a PR against the 5.0 rc? Seems like a good thing to try to get in.

wolfd added a commit to wolfd/bazel that referenced this issue Nov 23, 2021
JSON persistent workers now delimit requests with newlines (`\n` on Unix, `\r\n` on Windows).

Fixes bazelbuild#14218.

Closes bazelbuild#14219.

PiperOrigin-RevId: 411596359
@wolfd
Copy link
Contributor Author

wolfd commented Nov 23, 2021

@wolfd Do you want to make a PR against the 5.0 rc? Seems like a good thing to try to get in.

Sure: #14314

meteorcloudy pushed a commit that referenced this issue Nov 24, 2021
JSON persistent workers now delimit requests with newlines (`\n` on Unix, `\r\n` on Windows).

Fixes #14218.

Closes #14219.

PiperOrigin-RevId: 411596359
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants