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

Please consider exposing WorkRequestHandler to workspaces #14556

Open
bnavetta opened this issue Jan 11, 2022 · 8 comments
Open

Please consider exposing WorkRequestHandler to workspaces #14556

bnavetta opened this issue Jan 11, 2022 · 8 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Local-Exec Issues and PRs for the Execution (Local) team type: feature request

Comments

@bnavetta
Copy link

Description of the problem / feature request:

I'd like to be able to reuse WorkRequestHandler for implementing custom persistent workers.

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

The com.google.devtools.build.lib.worker.WorkRequestHandler class is a great starting point for implementing persistent workers. However, external workers can't use it because it's inside the Bazel codebase and not exported under @bazel_tools. It'd be easier to write custom workers if they could reuse WorkRequestHandler.

What operating system are you running Bazel on?

macOS Big Sur

What's the output of bazel info release?

INFO: Invocation ID: 49fb37be-a171-4419-a1b5-a2078ef9f44b
WARNING: info command does not support starlark options. Ignoring options: [--@io_bazel_rules_scala//scala/settings:stamp_scala_import=False]
release 4.2.1

Have you found anything relevant by searching the web?

Any other information, logs, or outputs that you want to share?

bazel query @bazel_tools//... shows other built-in tools that rules can reuse, like singlejar and the work request proto definitions at @bazel_tools//src/main/protobuf:worker_protocol_proto. Based on the location that worker_protocol_proto is at, I spent a while trying to depend on @bazel_tools//src/main/java/com/google/devtools/build/lib/worker to work.

@larsrc-google
Copy link
Contributor

We don't want to add more things to @bazel_tools, see #4301.

@dieu
Copy link

dieu commented Jan 12, 2022

@larsrc-google what do you think about publishing part of Bazel library as independent maven artifacts? Actually, it is less about @bazel_tools, more about not copy-paste code from Bazel repo to do persistent worker properly.

@sventiffe sventiffe added team-Local-Exec Issues and PRs for the Execution (Local) team untriaged labels Jan 13, 2022
@larsrc-google
Copy link
Contributor

It's a good idea, and we're now talking internally about what the best way to do this kind of API-like thing is. There will be some maintenance involved if we separate it out.

@meisterT meisterT added P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels Apr 7, 2022
@Bencodes
Copy link
Contributor

It's a good idea, and we're now talking internally about what the best way to do this kind of API-like thing is. There will be some maintenance involved if we separate it out.

@larsrc-google We chatted briefly after the Performance BOF session. Is this something you'd still be willing to do?

@Wyverald
Copy link
Member

I think the reluctance to add stuff to @bazel_tools is mainly around having to tie fixes to the "stuff" to Bazel releases. So we could release this WorkRequestHandler thing as (part of?) a library that's separately packaged and released, but on the flip side, the thing to watch out for would be version mismatches between the running Bazel version (the "server" of this protocol, if I understand correctly), and this library that users are using. Maybe using a proto/grpc interface would be better?

@larsrc-google
Copy link
Contributor

Just for clarification: Is it impossible to depend on WorkRequestHandler, or is it just problematic because it pulls in much more than it ought to? If the former, that should be fixed, it's definitely intended to be depended upon.

@bnavetta
Copy link
Author

I'm no longer working on that codebase, but I think it's currently impossible to depend on WorkRequestHandler outside of Bazel itself. Even if a workspace were to pull in the Bazel repository as a dependency, the visibility for //src/main/java/com/google/devtools/build/lib/worker:work_request_handlers, which defines WorkRequestHandler, is //src:__subpackages__, so it can't be depended on

@larsrc-google
Copy link
Contributor

:head_palm: Making it public.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Local-Exec Issues and PRs for the Execution (Local) team type: feature request
Projects
None yet
Development

No branches or pull requests

9 participants