Hello! Thanks for the pull request. A few questions and whatnot:
What sort of use cases is this for? If you want to run a command with a specific environment variable, there's already the update_env keyword argument.
The tests don't seem to cover all the functionality added. For instance, you haven't added any tests that the functionality works over SSH.
Any particular reason for writing your test using unittest, rather than using nose as the other tests do?
In the test that you added, asserting that the value retrieved is not None is a fairly weak assertion. Perhaps try setting an environment variable and then verifying that you can retrieve that value?
setenv doesn't seem to work, since export is a shell command rather than an actual program.
Even if setenv worked, I don't think it would not play nicely with retrieving environment variables, since you're caching the value of all environment variables on first retrieval.
My preference would be to make env a function rather than a property, since it's potentially doing significant work (for instance, sending commands over SSH).
The code uses underscores rather than camelCase since that's what PEP 8 specifies.
Hello! Thanks for the pull request. A few questions and whatnot:
update_env
keyword argument.unittest
, rather than usingnose
as the other tests do?None
is a fairly weak assertion. Perhaps try setting an environment variable and then verifying that you can retrieve that value?setenv
doesn't seem to work, sinceexport
is a shell command rather than an actual program.setenv
worked, I don't think it would not play nicely with retrieving environment variables, since you're caching the value of all environment variables on first retrieval.env
a function rather than a property, since it's potentially doing significant work (for instance, sending commands over SSH).