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

Convert bash script to Python #15

Conversation

texasaggie97
Copy link
Collaborator

@texasaggie97 texasaggie97 commented Aug 30, 2024

Summary of Changes

  • Uses python3 and included packages only
  • Designed to be easy to add additional configurations
  • Configure complete
  • Verify complete
  • Similar functionality to the bash script
  • Add --dry-run parameter - will not make any changes to the system (both installing/removing packages and updating config files)

Justification

Testing

Manual run with nilrt

Procedure

  • This PR: changes user-visible behavior, fixes a bug, or impacts the project's security profile; and so it includes a CHANGELOG note.
  • I certify that the contents of this pull request complies with the Developer Certificate of Origin.

@texasaggie97 texasaggie97 force-pushed the users/texasaggie97/convert-script-to-python branch 2 times, most recently from 1de463c to 97f27c6 Compare September 3, 2024 18:33
.gitignore Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
nilrt_snac/__main__.py Outdated Show resolved Hide resolved
nilrt_snac/__main__.py Outdated Show resolved Hide resolved
nilrt_snac/_Configs/_NiauthConfig.py Outdated Show resolved Hide resolved
nilrt_snac/_Configs/_NiauthConfig.py Outdated Show resolved Hide resolved
nilrt_snac/_Configs/_NtpConfig.py Outdated Show resolved Hide resolved
nilrt_snac/_Configs/_NtpConfig.py Outdated Show resolved Hide resolved
nilrt_snac/_Configs/_OpkgConfig.py Outdated Show resolved Hide resolved
@texasaggie97 texasaggie97 force-pushed the users/texasaggie97/convert-script-to-python branch from 97f27c6 to a0101f9 Compare September 3, 2024 20:26
@texasaggie97 texasaggie97 force-pushed the users/texasaggie97/convert-script-to-python branch 5 times, most recently from bbbf080 to 9a7ab4a Compare September 5, 2024 15:21
@texasaggie97
Copy link
Collaborator Author

Patch V2

  • All review comments addressed either by fixing or explaining
  • Follow naming conventions consistently
    • Files names changed to lower case and snake_case
    • SnacError -> SNACError
    • Configs -> CONFIGS
  • Moves _configure() and _verify() to __main__.py instead of in their own files
  • logger.info() changed to print
  • "verbosity" simplified

@AlexHearnNI AlexHearnNI requested a review from a team September 5, 2024 17:19
nilrt_snac/__main__.py Outdated Show resolved Hide resolved
src/nilrt-snac Outdated Show resolved Hide resolved
src/nilrt-snac Outdated Show resolved Hide resolved
src/nilrt-snac Outdated Show resolved Hide resolved
nilrt_snac/__init__.py Outdated Show resolved Hide resolved
nilrt_snac/__init__.py Outdated Show resolved Hide resolved
nilrt_snac/__main__.py Show resolved Hide resolved
@amstewart
Copy link
Collaborator

@texasaggie97 I just pulled #3. So you may need to rebase this PR on the latest master.

nilrt_snac/requirements.txt Outdated Show resolved Hide resolved
nilrt_snac/__main__.py Show resolved Hide resolved
@texasaggie97 texasaggie97 force-pushed the users/texasaggie97/convert-script-to-python branch 3 times, most recently from e5616e7 to 5488d5a Compare September 6, 2024 15:16
@texasaggie97
Copy link
Collaborator Author

Patch v3

  • Include faillock config (converted from bash)
  • Addition review comments
    • Add --version parameter
    • Remove requirements.txt file
    • Change Errors to inherit from IntEnum instead of Enum
  • Change verify() to raise an exception when it finds an issue

@amstewart
Copy link
Collaborator

@texasaggie97 Are you going to update the _Config class names?

* Uses python3 and included packages only
* Designed to be easy to add additional configurations
* Configure complete
* Verify complete
* Similar functionality to the bash script

Signed-off-by: Mark Silva <[email protected]>
@texasaggie97 texasaggie97 force-pushed the users/texasaggie97/convert-script-to-python branch from dd8b58c to 29ad882 Compare September 6, 2024 18:58
@amstewart amstewart merged commit 093a687 into ni:master Sep 6, 2024
1 check passed
@texasaggie97 texasaggie97 deleted the users/texasaggie97/convert-script-to-python branch September 6, 2024 20:08
@amstewart amstewart mentioned this pull request Sep 9, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants