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

Make rebench-denoise more robust to absent tools and running as root with a user-level installation #260

Merged
merged 10 commits into from
Sep 4, 2024

Conversation

smarr
Copy link
Owner

@smarr smarr commented Sep 3, 2024

The perhaps most important issue this PR addresses is rebench-denoise being used with sudo but root not having access to any of the installed packages and dependencies.

To avoid the issue in the first place, we split the denoise implementation and move all use of packages into the part that's only used from rebench.
We also set PYTHONPATH so that denoise finds itself.

More minor improvements include:

  • more robust to missing tools like cset, and giving more useful error
  • fix the error reporting for denoise to actually render the nice overview of what didn't work

This resolves #251.

@smarr smarr added the Feature label Sep 3, 2024
@coveralls
Copy link

coveralls commented Sep 3, 2024

Coverage Status

coverage: 53.929% (-31.5%) from 85.47%
when pulling cb67b63 on redhat-which
into 563af03 on master.

- give more useful errors when tools are not available and how to work around it
- don’t error when cset not available

Signed-off-by: Stefan Marr <[email protected]>
Signed-off-by: Stefan Marr <[email protected]>
- preserve PYTHONPATH across sudo for denoise
- adapt tests to handle the PYTHONPATH that’s now set

Signed-off-by: Stefan Marr <[email protected]>
The main goal here is to have denoise require the fewest possible dependencies when running under root, which avoids issues with PYTHONPATH, dependencies being installed by the user, and generally to minimize the capabilities of denoise.

- denoise_client now provides the interface for rebench to access denoise
- minor refactorings to avoid circular dependencies
- move getting number of cores to denoise_client to avoid dependency on cpuinfo inside root
- cache number of cpu cores to avoid requesting them repeatedly which is very expensive

Signed-off-by: Stefan Marr <[email protected]>
- remember when we didn’t find the path

Signed-off-by: Stefan Marr <[email protected]>
Since there are user-visible changes around the denoise interface and behavior, I am bumping the version.

Signed-off-by: Stefan Marr <[email protected]>
@smarr smarr merged commit 48374b9 into master Sep 4, 2024
10 checks passed
@smarr smarr deleted the redhat-which branch September 4, 2024 11:57
@smarr smarr changed the title Make rebench-denoise more robust to absent tools and running as root but a user-level installation Make rebench-denoise more robust to absent tools and running as root with a user-level installation Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fail with useful error when cset is not available
2 participants