-
-
Notifications
You must be signed in to change notification settings - Fork 120
feat: expand the usefulness of resource_sets #1235
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
base: main
Are you sure you want to change the base?
Conversation
f9d0edb to
e6a1d98
Compare
e6a1d98 to
4eab914
Compare
|
This adds a new public function, resource_set_for, which takes a cpu and memory value and returns an appropriate resource_set function for those values. The bazel resource_set API isn't very easy to work with, and this hides the ugliness here, rather than expecting every module to implement something like [rules_rust did](https://github.com/bazelbuild/rules_rust/blob/f29a63cb3c473bd0158c8c9d3e0793a33187d505/rust/private/rustc_resource_set.bzl) This implementation is ugly, but it's made slightly less bad by using a helper script to generate it, and I reason that people attempting to use the resource_set API would be better off with additional abstraction, so if/when this gets fixed in bazel, most people don't need to notice.
4eab914 to
ca0fe9e
Compare
|
As a follow-up, here are some of the other things I was looking at doing:
I'm interested in helping achieve cross-consistency for these behaviors, which means I'd like (at least) 1 to be handled in bazel-lib, and maybe 2 if there's consensus. |

This adds a new public function,
resource_set_for, which takes a cpu and memory value and returns an appropriateresource_setfunction for those values, adding some support for the cross-product mentioned in the previous pass. The bazel resource_set API isn't very easy to work with, and this hides the ugliness here, rather than expecting every module to implement something like rules_rust did.The ugliness of the implementation is mitigated slightly by using a helper script to generate it, and I reason that people attempting to use the
resource_setAPI would be better off with additional abstraction, so if/when this gets fixed in bazel, most people don't need to notice.