alan-turing-institute / data-safe-haven

https://data-safe-haven.readthedocs.io
BSD 3-Clause "New" or "Revised" License
61 stars 15 forks source link

Fix Pulumi/dsh Python mismatch #2240

Closed jemrobinson closed 1 month ago

jemrobinson commented 1 month ago

:white_check_mark: Checklist

:vertical_traffic_light: Depends on

n/a

:arrow_heading_up: Summary

Set PULUMI_PYTHON_CMD to use the same Python executable that is being used by dsh.

Thanks for the pointer @craddm !

:closed_umbrella: Related issues

Closes #2239

:microscope: Tests

Tested locally, but we should test on the rc1 branch once this is merged into it

github-actions[bot] commented 1 month ago

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  data_safe_haven/external/interface
  pulumi_account.py
Project Total  

This report was generated by python-coverage-comment-action

craddm commented 1 month ago

Seems to work on my Docker container

jemrobinson commented 1 month ago

Yeah, it also worked for me. I'm not sure why, but I assume it sets the path correctly behind the scenes?

JimMadge commented 1 month ago

Interesting, maybe the interpreter "knows" where it's modules are, or the default path is relative to the executable file or something. If this works on its own though I don't see much benefit in also finding and setting the venv path :+1:.

jemrobinson commented 1 month ago

I tried the venv method and couldn't get it to work, so if this fix works for @helendduncan then I'm happy with it :)

JimMadge commented 1 month ago

Seems to work on my Docker container

Reading this a second time, does that mean this works in an environment where it didn't work before (a pipx install), or it still works in your container where it worked previously?

craddm commented 1 month ago

Seems to work on my Docker container

Reading this a second time, does that mean this works in an environment where it didn't work before (a pipx install), or it still works in your container where it worked previously?

pipx didn't work previously - same issue with not finding the modules. Note this is standalone Docker container, not the .devcontainer that I usually use.

JimMadge commented 1 month ago

Great, that sounds like a positive result for this change working then :tada:.

jemrobinson commented 1 month ago

Confirmed working by @helendduncan