Skip to content

feat: add a helper for rules to work with resource_sets#792

Merged
alexeagle merged 2 commits intomainfrom
resource_sets
Mar 18, 2024
Merged

feat: add a helper for rules to work with resource_sets#792
alexeagle merged 2 commits intomainfrom
resource_sets

Conversation

@alexeagle
Copy link
Copy Markdown
Collaborator

This API is poorly designed and needs some help to let rule users pick a value in cases that they aren't also the rule author

Workaround for bazelbuild/bazel#15187


Type of change

  • New feature or functionality (change which adds functionality)

For changes visible to end-users

  • Relevant documentation has been updated

Test plan

  • Manual testing; please provide instructions so we can reproduce:
    I'm using it downstream in rules_ts.

This API is poorly designed and needs some help to let rule users pick a value in cases that they aren't also the rule author
@alexeagle alexeagle requested review from jbedard and thesayyn March 15, 2024 21:28
return {"cpu": 2}

def _resource_set_cpu_4(_, __):
return {"cpu": 4}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you're using 4 cores it's Likely you are using more than 250mB as well. So maybe you want a cross-product? This bazel API is silly

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alexeagle pushed a commit that referenced this pull request Jan 23, 2026
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, adding some support for the cross-product [mentioned in
the previous
pass](#792 (comment)).
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).

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_set` API would be better off with additional abstraction,
so if/when this gets fixed in bazel, most people don't need to notice.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants