Adding authselect common configurations#256
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for creating and configuring custom Authselect profiles in the test framework. It updates AuthselectUtils to accept a filesystem utility, handle cleanup of custom profiles, and adds AuthselectCommonConfiguration to set up common PAM configurations like offline logins and SSSD domains. The review feedback suggests removing a redundant runtime import and an unused self statement, stripping leading indentation from the multiline PAM stack strings before writing them to files, and renaming the loop variable files to a singular form for better clarity.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| "AuthselectUtils", | ||
| ] | ||
|
|
||
| from pytest_mh.utils.fs import LinuxFileSystem |
There was a problem hiding this comment.
Since from __future__ import annotations is used and LinuxFileSystem is only used as a type hint, the runtime import is redundant. It is already imported under if TYPE_CHECKING: on line 10, so this runtime import can be safely removed.
|
|
||
| self |
There was a problem hiding this comment.
The statement self on its own line is redundant and has no effect. It should be removed.
| session required pam_unix.so | ||
| session optional pam_sss.so forward_pass | ||
| """ | ||
| if not pam_files: |
There was a problem hiding this comment.
The multiline string stack contains leading indentation for each line. When written to the PAM configuration files, this leading whitespace will be preserved, making the files messy. It is recommended to strip the leading indentation from each line before writing.
stack = '\\n'.join(line.strip() for line in stack.splitlines()).strip()\n if not pam_files:| for files in pam_files: | ||
| self.authselect.fs.write(f"{self.profile_path}/{profile_name}/{files}", stack) |
There was a problem hiding this comment.
The loop variable files is plural, but it represents a single file name in each iteration. It should be singular (e.g., pam_file or file_name) to improve readability and avoid confusion.
for pam_file in pam_files:\n self.authselect.fs.write(f'{self.profile_path}/{profile_name}/{pam_file}', stack)| session required pam_unix.so | ||
| session optional pam_sss.so | ||
| """ | ||
| if not pam_files: |
There was a problem hiding this comment.
The multiline string stack contains leading indentation for each line. When written to the PAM configuration files, this leading whitespace will be preserved, making the files messy. It is recommended to strip the leading indentation from each line before writing.
stack = '\\n'.join(line.strip() for line in stack.splitlines()).strip()\n if not pam_files:| for files in pam_files: | ||
| self.authselect.fs.write(f"{self.profile_path}/{profile_name}/{files}", stack) |
There was a problem hiding this comment.
The loop variable files is plural, but it represents a single file name in each iteration. It should be singular (e.g., pam_file or file_name) to improve readability and avoid confusion.
for pam_file in pam_files:\n self.authselect.fs.write(f'{self.profile_path}/{profile_name}/{pam_file}', stack)
9c9c284 to
c57dfa0
Compare
1 participant
No description provided.