-
-
Notifications
You must be signed in to change notification settings - Fork 941
Get system user id in a lazy manner #1072
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Get system user id in a lazy manner #1072
Conversation
a4bd856
to
4598e99
Compare
Calling getpass.getuser may lead to breakage in environments where there is no entries in the /etc/passwd file for the current user. Setting the environment variables for the git user configurations should prevents GitPython from using values from /etc/passwd. However, doing so will not prevent reading /etc/passwd and looking for an entry with the current user UID. This patch changes the behavior described above so GitPython will perform a lazy evaluation of /etc/passwd, only doing so when the environment variables for the git user configuration are not available. Signed-off-by: Athos Ribeiro <athos@redhat.com>
4598e99
to
100fe91
Compare
GitPython will check the git user related environment variables when setting reflogs on submodule updates. This patch sets such variables so Cachito does not need to rely on the values in the passwd file. Moreover, with gitpython-developers/GitPython#1072, values in /etc/passwd will not be checked at all. Signed-off-by: Athos Ribeiro <athos@redhat.com>
GitPython will check the git user related environment variables when setting reflogs on submodule updates. This patch sets such variables so Cachito does not need to rely on the values in the passwd file. Moreover, with gitpython-developers/GitPython#1072, values in /etc/passwd will not be checked at all. Signed-off-by: Athos Ribeiro <athos@redhat.com>
Great work, thanks a lot! It's also my first time to see the If this issue is holding you back, I am happy to create a new patch release if you request it. |
I would really appreciate that! As soon as that release is out, I will work to push that to Fedora (that is where I consume the package from). Thank you :) |
Alright, v3.1.10 was just released to pypi. |
Thanks for that! Note that a new tag was not pushed to this repository yet. |
Thanks for the reminder. Done.
… On 23. Oct 2020, at 16:33, Athos Ribeiro ***@***.***> wrote:
Thanks for that!
Note that a new tag was not pushed to this repository yet.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub <#1072 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAPRBWBXNQAQRJ5ZOZP3U3SME5VZANCNFSM4SYWCHHQ>.
|
That was fast! I was about to update my comment to say https://gitpython.readthedocs.io/en/stable/changes.html still needs updating. Let me know if you need any help with that. I am building the new Fedora packages right now. Thanks again for the quick replies on this one!!! |
return user_id | ||
|
||
def default_name(): | ||
default_email().split('@')[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@athos-ribeiro @Byron
(not sure if I'm really correct) Is return
missing here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was investigating an error in our CI and found this patch which is included in gitpython==3.10.0
and seems related to the error.
https://github.com/mlflow/mlflow/pull/3584/checks?check_run_id=1297036420#step:4:1677
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guess I messed up on this one :)
Will provide a follow-up fix right away.
I am sorry for this :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no problem :) Thanks for the quick reponse!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks everyone. With the next PR to fix this one you will get a new patch release right away.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixing it in #1073.
Once again, sorry for the inconvenient
GitPython will check the git user related environment variables when setting reflogs on submodule updates. This patch sets such variables so Cachito does not need to rely on the values in the passwd file. Moreover, with gitpython-developers/GitPython#1072, values in /etc/passwd will not be checked at all. Signed-off-by: Athos Ribeiro <athos@redhat.com>
GitPython will check the git user related environment variables when setting reflogs on submodule updates. This patch sets such variables so Cachito does not need to rely on the values in the passwd file. Moreover, with gitpython-developers/GitPython#1072, values in /etc/passwd will not be checked at all. Signed-off-by: Athos Ribeiro <athos@redhat.com>
Calling getpass.getuser may lead to breakage in environments where there
is no entries in the /etc/passwd file for the current user.
Setting the environment variables for the git user configurations should
prevents GitPython from using values from /etc/passwd. However, doing so
will not prevent reading /etc/passwd and looking for an entry with the
current user UID.
This patch changes the behavior described above so GitPython will
perform a lazy evaluation of /etc/passwd, only doing so when the
environment variables for the git user configuration are not available.
Signed-off-by: Athos Ribeiro athos@redhat.com