search

DiamondLightSource / dodal

Ophyd devices and other utils that could be used across DLS beamlines
Apache License 2.0
2 stars 8 forks source link

Make CLI beamline mappings work without mutating environment #569

Open callumforrester opened 3 months ago

callumforrester commented 3 months ago 
          Setting `os.environ["BEAMLINE"]` caused unintended side effects, will make an issue to find a better way

_Originally posted by @callumforrester in https://github.com/DiamondLightSource/dodal/pull/519#discussion_r1609836803_

callumforrester commented 3 months ago

The CLI mutates the $BEAMLINE environment variable to keep the mappings between the beamline name and the module name consistent. For example if a user types dodal connect s03, dodal knows it must use the i03 module, but the i03 module depends on $BEAMLINE being s03 to connect to the s03 PVs.

We need to find a cleaner way of doing this that doesn't force us to keep the CLI argument and the environment variable in sync.

May be related to #502, as a solution to that could supersede the $BEAMLINE variable.

Another possible solution is to have a BEAMLINE global that is informed by the CLI argument if supplied, and the environment variable if not, and have that passed universally to all parts of the code including the beamline module.

DominicOram commented 3 months ago

Setting os.environ["BEAMLINE"] caused unintended side effects

Can you elaborate on the side effects? In general though I agree that forcing the CLI arg and the env variable to be in sync is messy so happy to look for a better way.

callumforrester commented 3 months ago

The main one I saw was when I called it in the tests, it could change the behaviour of other tests that called code that read ${BEAMLINE}, for example: https://github.com/DiamondLightSource/dodal/blob/4f1376592e8e2a155b903147e869efe82c3b8288/tests/unit_tests/test_log.py#L88

I had to make sure os.environ was patched for every CLI test, which is a reasonable solution, but as you say it is messy and generally good practice to avoid mutating global state because things like that pop up and can be hard to diagnose.