Remote execution, exec properties, and allowed keys

563 views
Skip to first unread message

John Cater

unread,
Dec 5, 2022, 12:44:13 PM12/5/22
to bazel-discuss
In Bazel, the "execution requirements" and "execution properties" are similar but different mechanisms for passing data to the execution strategy (which in practice almost always means they configure the remote executor). There are a number of asymmetries here and we should unify the systems, but that's a larger-scale project for a later day.

One in particular, though, are the allowed keys. Specifically, when tags are converted to execution requirements, the code in TagUtils.legalExecInfoKeys is used to check if the value is allowed.

There is no similar check for execution property keys, however, which means that a user can use exec_properties to pass any key they wish to the remote executor.

I don't actually know if this is a problem! Maybe we want to filter tags, because tags cover more things than just execution requirements, and we don't want to send things like "noci" to the remote executor. Or maybe we should filter the exec properties, because we don't want users to be able to override execution timeouts (as an example), we only want an allowed list of things users can affect.

So, people interested in remote execution, what are your thoughts? What's the right behavior here? I tested a change that adds this filter for execution properties (https://github.com/bazelbuild/bazel/pull/16902), but I'm not sure this is the right thing to do, so I'd like some advice.

Thanks!
John Cater

william...@gmail.com

unread,
Dec 8, 2022, 12:39:44 PM12/8/22
to bazel-discuss
Hi John,

Consider this a vote for keeping exec_properties around and preserving the ability to include free-form tags like today. We use exec_properties extensively in BuildBuddy to configure various features of the remote execution system. I believe we got this idea from the RBE alpha which also did.

Because remote exec implementations vary widely and are not controlled by Bazel, it seems ideal to allow a "pass-through" API here.

Happy to discuss further!

Thanks,

Tyler

Yannic Bonenberger

unread,
Dec 8, 2022, 1:54:30 PM12/8/22
to william...@gmail.com, bazel-discuss
Definitely +1 on that. EngFlow also makes extensive use of exec_properties to configure various features on a per-target base if needed.

I’ve also fallen into the pitfall that execution_requirements from ctx.actions.run and exec_properties from on targets behave differently. I would actually love if arbitrary keys would be allowed in execution_requirements, or that Bazel would at least print a warning when it drops them instead of doing that silently.

Thanks,
Yannic

-- 
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/972a99e8-cccb-4177-890b-e0fcd8aaf2b8n%40googlegroups.com.

John Cater

unread,
Dec 20, 2022, 11:31:48 AM12/20/22
to Yannic Bonenberger, william...@gmail.com, bazel-discuss
Thanks for the comments. I'll definitely note that we need to keep this functionality for Bazel users.

Reply all
Reply to author
Forward
0 new messages