Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DTT1. Change the call to os.getlogin function by getpass.getuser in testing module #5203

Closed
mhamra opened this issue Apr 11, 2024 · 4 comments · Fixed by #5211
Closed

DTT1. Change the call to os.getlogin function by getpass.getuser in testing module #5203

mhamra opened this issue Apr 11, 2024 · 4 comments · Fixed by #5211
Assignees
Labels

Comments

@mhamra
Copy link
Member

mhamra commented Apr 11, 2024

Target version Related issue Related PR/dev branch
4.9.0 #5193

Description

While executing a workflow file, the os.getlogin() function generated an exception. See this comment.

The os.getlogin function returns the user's name logged in on the process's controlling terminal. Typically, processes in the user's session (tty, X session) have a controlling terminal. Processes spawned by the workflow do not have a controlling terminal. The recommended way to obtain the current user is using the getpass.getuser function instead. It is a more reliable method.

@rauldpm
Copy link
Member

rauldpm commented Apr 11, 2024

Mmmm, looking at the command used in the comment, I do not have any error using it; it may be your Python/GLIB permission.

image

In any case, @fcaffieri let's set the issue as a type/change and work on it in DTT2, what do you think?

https://stackoverflow.com/questions/74187896/os-getlogin-in-docker-container-throws-filenotfounderror

@mhamra
Copy link
Member Author

mhamra commented Apr 11, 2024

The Python reference for os.getlogin recommends using the getpass.getuser function (see os.getlogin reference).

I recommend this change because some users may get the exception, blocking the test execution.

@rauldpm
Copy link
Member

rauldpm commented Apr 11, 2024

There is only this reference to getlogin, so let's fix it, as you have the fix, create a PR to the #5190 branch and add a test case

╰─➤  grep -R "os.getlogin"
modules/testing/testing.py:        extra_vars['current_user'] = os.getlogin()

@rauldpm
Copy link
Member

rauldpm commented Apr 12, 2024

Approved by @pro-akim and @rauldpm

@rauldpm rauldpm closed this as completed Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants