Skip to content

Convert bash script to Python #15

Merged
amstewart merged 1 commit intoni:masterfrom
texasaggie97:users/texasaggie97/convert-script-to-python
Sep 6, 2024
Merged

Convert bash script to Python #15
amstewart merged 1 commit intoni:masterfrom
texasaggie97:users/texasaggie97/convert-script-to-python

Conversation

@texasaggie97
Copy link
Copy Markdown
Collaborator

@texasaggie97 texasaggie97 commented Aug 30, 2024 •
edited
Loading

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
amstewart
amstewart requested changes Sep 3, 2024
View reviewed changes
@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
Copy Markdown
Collaborator Author

texasaggie97 commented Sep 5, 2024

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
dmondrik
dmondrik reviewed Sep 6, 2024
View reviewed changes
dmondrik
dmondrik reviewed Sep 6, 2024
View reviewed changes
dmondrik
dmondrik reviewed Sep 6, 2024
View reviewed changes
amstewart
amstewart requested changes Sep 6, 2024
View reviewed changes
@amstewart
Copy link
Copy Markdown
Collaborator

amstewart commented Sep 6, 2024

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

amstewart
amstewart requested changes Sep 6, 2024
View reviewed changes
amstewart
amstewart requested changes Sep 6, 2024
View reviewed changes
@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
Copy Markdown
Collaborator Author

texasaggie97 commented Sep 6, 2024

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
Copy Markdown
Collaborator

amstewart commented Sep 6, 2024

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

@amstewart amstewart mentioned this pull request Sep 6, 2024
Merged
6 tasks
@texasaggie97
Convert bash script to Python: AB#2831708
29ad882 
* 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 <mark.silva@ni.com>
@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
@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
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dmondrik dmondrik dmondrik left review comments

@amstewart amstewart amstewart approved these changes

@AlexHearnNI AlexHearnNI Awaiting requested review from AlexHearnNI AlexHearnNI is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@texasaggie97 @amstewart @dmondrik