Do not call get_user_id if it is not needed #1314
Conversation
On systems without any environment variables and no pwd module, gitpython crashes as it tries to read the environment variable before looking at its config.
|
It looks like a maintainer might have to approve the workflow runs each time I try to fix the mypy troubles... |
|
I've added an overload to config.py get_value() in main that fixes the/a mypy issue, if you want to copy that accross edit: |
This won't try and do something silly like convert `username=1` to a number.
No you didn't, you closed it! |
| self.assertTrue(committer.email.startswith('user@')) | ||
| committer = Actor.committer(None) | ||
| author = Actor.author(None) | ||
| # We can't test with `self.rorepo.config_reader()` here, as the uuid laziness |
There was a problem hiding this comment.
I assume there's some way to get a writeable repo that we can override the config in, but I'm not familiar enough with your fixtures to want to go to the effort of working that out.
There was a problem hiding this comment.
Thanks a lot for helping with the test. Getting a repository with write permissions can be done like this.
There was a problem hiding this comment.
Thanks, I added another test. It turns out that this comment still applies, as the only reason now that the patched get_user would be called is if the global git config is missing.
There was a problem hiding this comment.
Just wondering, does it matter if Dev's have to have git.name --global set?
I just did a fresh Ubuntu install and one test failed until I set git.email anyway.
If that wasn't a fluke, can put in test instructions to set both.
There was a problem hiding this comment.
Which test fails?
There was a problem hiding this comment.
Test_remote.py, assertion at line 333 failed.
Also came with warnings about not being able to do something (create temporary file/folder?) at git://localhost:19418/daemon_repo-test_base-5tjbtmwj.
But fixed by setting git.email --global
Could have been an anomaly, too much work to reinstall and build python from source again to check!
There was a problem hiding this comment.
Oh, I thought you meant my new test failed!
On systems without any environment variables (such as sandboxed CI runs) and no
pwdmodule (such as windows), GitPython crashes.This is just a logic error - there is no reason to fallback to
pwduntil the config lookup has actually failed. The implementation ofConfigWriter.get_valueis to catchException, so this patch doesn't attempt to work out a more specific exception than what was used before.This fixes #356