psf / psf/pyperf

Allow setting updated/new env var *values* on Runner.

Open
#100 2 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
962
Forks
106
Avg merge
2d 18h
Merged PRs (30d)
4

Description

In _utils.py the create_environ() helper produces a dict that can be passed as the "env" arg to subprocess.run(), etc. It is used in Master.spawn_worker() (in _master.py). Currently it takes an "inherit_environ" arg (Bool) that corresponds to the "--inherit-environ" flag in the Runner CLI (see Runner.init() in _runner.py). This results in a potentially problematic situation.

The Problem

Let's say you have a benchmark script that internally relies on some environment variable that is defined relative to the commandline args given to the script. This environment variable may be set already or it might not. Regardless, you will be setting it to some new value. To make this work you need to do something like the following:

    # This is a concrete example.
    os.environ["MY_VAR"] = "spam" if runner.args.my_flag else "eggs"
    if runner.args.inherit_environ is None:
        runner.args.inherit_environ = ["MY_VAR"]
    else:
        runner.args.inherit_environ.append("MY_VAR")

However, in some cases you can't leave the env var set (or maybe the env var could cause pyperf to break). Plus things are more complicated if you have more than one such env var.

The Solution

Consequently, in a benchmark script it would be nice to be able to give the actual env var pairs to Runner rather than doing the dance above. Here are some possible approaches to solve the problem:

  • allow Runner.args.inherit_environ to be a dict
    • create_environ() would be updated to do the right thing
    • this is probably too messy to be part of pyperf's public API
  • add something like Runner.env_vars to allow benchmarks to explicitly set env var values to be used in workers
    • create_environ() would grow a new "env" arg or "inherit_environ" would updated as above
  • add Runner.add_env_var(name, value=INHERIT)
    • this is like either of the two above, but probably a better public API for the functionality

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading create_environ() in _utils.py, its use in Master.spawn_worker() in _master.py, and Runner.init() in _runner.py. Decide how Runner should accept explicit environment values, then ensure workers receive those values without requiring benchmark code to mutate os.environ; the existing inherit-environ behavior should remain accounted for.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
performance, tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.