Cirrus: Omit functions in env. file #17458
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
openshift-ci
bot
added
do-not-merge/work-in-progress
|
LGTM |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cevich, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
Sorry dan, not quite ready yet. I'll un-draft this when it's complete. |
cevich
changed the title
cevich
force-pushed
the
fix_benchmarks_env
branch
from
d6aa21a to
d16a28f
Compare
The `localbenchmarks()` function stores a `.env` file containing current environment variables for benchmark-classification purposes. However its naked use of `printenv` means it was logging the contents of library functions and (worse) trying to stort all the lines. This results in an unusable mess inside `benchmarks.env`. Fix this by re-using the purpose-built passthrough_envars() which is designed to only print useful, safe, env. vars. Signed-off-by: Chris Evich <[email protected]>
cevich
force-pushed
the
fix_benchmarks_env
branch
from
d16a28f to
655441f
Compare
The `benchmarks.env` file is intended for machine consumption. Including things like a `kB` unit label (like `$MEMTOTAL`) make items difficult to parse. Additionally, multi-value keys (like `$UNAME_RM`) make extra/unnecessary work for the interpreter. Simplify these items and include a data-schema version marker so an interpreter can be made aware/support future format changes. Signed-off-by: Chris Evich <[email protected]>
cevich
force-pushed
the
fix_benchmarks_env
branch
from
655441f to
45fb353
Compare
cevich
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
This is ready now. |
|
@containers/podman-maintainers PTAL - this is needed so we can capture intelligible environment details along with the benchmark data. |
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
localbenchmarks()function stores a.envfile containing current environment variables for benchmark-classification purposes. However its naked use ofprintenvmeans it was logging the contents of library functions and (worse) trying to stort all the lines. This results in an unusable mess insidebenchmarks.env. Fix this by re-using the purpose-built passthrough_envars() which is designed to only print useful, safe, env. vars.Also
The
benchmarks.envfile is intended for machine consumption. Including things like akBunit label (like$MEMTOTAL) make items difficult to parse. Additionally, multi-value keys (like$UNAME_RM) make extra/unnecessary work for the interpreter. Simplify these items and include a data-schema version marker so an interpreter can be made aware/support future format changes.Does this PR introduce a user-facing change?