Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAdd 'sshkey' context manager #242
Conversation
| self.set_environment(**old_env) | ||
|
|
||
| @contextmanager | ||
| def sshkey(self, sshkey_file): |
Byron
Jan 21, 2015
Member
For a moment I thought this method is to specific, but on the other hand, it makes setting ssh-keys (by using key-files) really easy.
Therefore the only thing I will do is see how this works on windows, and possibly put in an assertion to run on non-windows systems only.
For a moment I thought this method is to specific, but on the other hand, it makes setting ssh-keys (by using key-files) really easy.
Therefore the only thing I will do is see how this works on windows, and possibly put in an assertion to run on non-windows systems only.
| @@ -0,0 +1,9 @@ | |||
| #!/usr/bin/env python | |||
Byron
Jan 21, 2015
Member
Possibly the MANIFEST.in file needs adjustment to assure this file also gets distributed when making new releases through setuptools.
Possibly the MANIFEST.in file needs adjustment to assure this file also gets distributed when making new releases through setuptools.
| @@ -603,6 +610,73 @@ def execute(self, command, | |||
| else: | |||
| return stdout_value | |||
|
|
|||
| def set_environment(self, **kwargs): | |||
Byron
Jan 21, 2015
Member
From looking at the implementation, I'd be tempted to call this method update_environment. set_environment seems to be more like this:
res = self._environment
self._environment = kwargs
return res
From looking at the implementation, I'd be tempted to call this method update_environment. set_environment seems to be more like this:
res = self._environment
self._environment = kwargs
return res| return old_env | ||
|
|
||
| @contextmanager | ||
| def environment(self, **kwargs): |
Byron
Jan 21, 2015
Member
This is an interesting one ! When used as with repo.git.environment(FOO='bar'):, the name makes total sense. When I imagine to write a test-case, I'd probably want to query the current state of the custom environment like this too: repo.git.environment(), following the set_foo(), foo() convention.
Can you imagine a better name for environment() to get a context manger ? I know, it's sweet and short already and anything else will make it longer ... with altered_environment(), with changed_environment(), with custom_environment() ?
This is an interesting one ! When used as with repo.git.environment(FOO='bar'):, the name makes total sense. When I imagine to write a test-case, I'd probably want to query the current state of the custom environment like this too: repo.git.environment(), following the set_foo(), foo() convention.
Can you imagine a better name for environment() to get a context manger ? I know, it's sweet and short already and anything else will make it longer ... with altered_environment(), with changed_environment(), with custom_environment() ?
|
Thank you ! |
| @@ -153,3 +153,23 @@ def test_env_vars_passed_to_git(self): | |||
| editor = 'non_existant_editor' | |||
| with mock.patch.dict('os.environ', {'GIT_EDITOR': editor}): | |||
| assert self.git.var("GIT_EDITOR") == editor | |||
|
|
|||
| def test_environment(self): | |||
teeberg
Jan 22, 2015
Author
I don't see how I could test the sshkey context manager without accessing an actual repo. Also, how could I check whether the environment is actually passed into the git process? The former would cover the latter. Do you have ideas?
I don't see how I could test the sshkey context manager without accessing an actual repo. Also, how could I check whether the environment is actually passed into the git process? The former would cover the latter. Do you have ideas?
Byron
Jan 22, 2015
Member
Something that could work is to adjust the script for the test to actually fail with a distinct error code, and possibly emit something distinctive to STDERR.
That way, you could call the real git process, on a bogus git@server:foo/bar repo URL, and verify that it failed under your conditions. That can proof that the script was actually called.
The cool thing is that this would require you to put in a way to adjust the path the script handling the GIT_SSH, which could be done so that advanced users can either set it, or derive from the Git command implementation.
Something that could work is to adjust the script for the test to actually fail with a distinct error code, and possibly emit something distinctive to STDERR.
That way, you could call the real git process, on a bogus git@server:foo/bar repo URL, and verify that it failed under your conditions. That can proof that the script was actually called.
The cool thing is that this would require you to put in a way to adjust the path the script handling the GIT_SSH, which could be done so that advanced users can either set it, or derive from the Git command implementation.
|
You did it ! Note to self: Test this on windows and at least throw saying sshkey doesn't work, if it doesn't work Also I believe today will be the day, as I should get through my docs review, and prepare a new release. |
Byron
merged commit 6f03861
into
gitpython-developers:master
|
Before I forget: I had to remove |
|
Thanks for the note, I already saw it! :) I didn't realise it would be such a pain! I'll stick with my local SSH script then in the meantime. :-) Thanks for merging everything else in! |
Proposed implementation for #234
I have done some manual tests, but not set up automated tests yet. I will see how it fits into your suite tomorrow if you haven't implemented it yet. :)
Let me know if something's not quite in the right place.