Conversation
| for n in self.envars: | ||
| os.environ.pop(n) |
There was a problem hiding this comment.
Should you reset self.envars back to an empty set after this?
There was a problem hiding this comment.
I think it would be safe to do in a unit test but I can make this take a list of keys to remove instead?
There was a problem hiding this comment.
Wait, I can't read code. This will only remove envs that have been added by this class.
So doing this back to an empty set is ok, since it won't affect anything existing ones in os.environ
hvpy/tests/test_io.py
Outdated
| def test_set_url_env(env): | ||
| env.set("HELIOVIEWER_API_URL", "https://fake_env_url/") | ||
|
|
||
| from hvpy.config import Settings | ||
|
|
||
| assert Settings().api_url == "https://fake_env_url/" |
There was a problem hiding this comment.
Can we add one more assertion after this that if you call set_api_url it uses the url passed in instead of the environment?
There was a problem hiding this comment.
I can do that but I am not sure it would be useful here.
The set_api_url function will not change this specific url since this Setting is not affected by that by the LiveSettings.
|
I will admit this is kind of overkill for setting the base url. |
Codecov Report
@@ Coverage Diff @@
## main #44 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 12 13 +1
Lines 196 198 +2
=========================================
+ Hits 196 198 +2
Continue to review full report at Codecov.
|
3 participants
No description provided.