Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Some best-practices updates to the kickstart script#3691

Merged
rawlinp merged 3 commits into
apache:masterfrom
ocket8888:kickstart-best-practices
Sep 17, 2019
Merged

Some best-practices updates to the kickstart script#3691
rawlinp merged 3 commits into
apache:masterfrom
ocket8888:kickstart-best-practices

Conversation

@ocket8888
Copy link
Copy Markdown
Contributor

What does this PR (Pull Request) do?

  • This PR obsoletes CLOSED - Comparison using is when operands support __eq__ #3503
    This PR makes some best-practices updates to the script responsible for generating the "network line" for kickstart ISO creation. Puts parenthesis around print statement args, better implementation of str.format and str.join, replaces a check for identity equality with the proper check for value equality, removed an unused variable, checking for presence of a member of a set/dict with in/not in instead of deprecated methods and/or weird argument nesting, etc.

Which Traffic Control components are affected by this PR?

  • Traffic Ops

What is the best way to verify this PR?

Generate an ISO through a Traffic Ops instance that has the new version of the script, verify that it comes out correctly. The script has no tests, but you could also just check the output using Docker or a VM.

The following criteria are ALL met by this PR

  • This PR includes tests OR I have explained why tests are unnecessary
  • This PR includes documentation OR I have explained why documentation is unnecessary
  • This PR includes an update to CHANGELOG.md OR such an update is not necessary
  • This PR includes any and all required license headers
  • This PR ensures that database migration sequence is correct OR this PR does not include a database migration
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY

@asfgit
Copy link
Copy Markdown
Contributor

asfgit commented Jun 20, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3886/
Test PASSed.

@mitchell852 mitchell852 added improvement The functionality exists but it could be improved in some way. Traffic Ops related to Traffic Ops tech debt rework due to choosing easy/limited solution labels Jun 25, 2019
@ocket8888 ocket8888 force-pushed the kickstart-best-practices branch from d33aa91 to 369897c Compare September 4, 2019 14:56
@asf-ci
Copy link
Copy Markdown
Contributor

asf-ci commented Sep 4, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4196/
Test PASSed.

Comment thread misc/kickstart_create_network_line.py Outdated
Comment thread misc/kickstart_create_network_line.py Outdated
Comment thread misc/kickstart_create_network_line.py Outdated
Comment thread misc/kickstart_create_network_line.py Outdated
@ocket8888 ocket8888 force-pushed the kickstart-best-practices branch from 369897c to f12ce64 Compare September 13, 2019 22:14
@asf-ci
Copy link
Copy Markdown
Contributor

asf-ci commented Sep 13, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4255/
Test PASSed.

Copy link
Copy Markdown
Contributor

@rawlinp rawlinp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one's getting there, but the spaces between between print and (...) like print (...) are weirding me out.

@asf-ci
Copy link
Copy Markdown
Contributor

asf-ci commented Sep 16, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4257/
Test PASSed.

@rawlinp rawlinp merged commit 5b018e2 into apache:master Sep 17, 2019
@ocket8888 ocket8888 deleted the kickstart-best-practices branch September 17, 2019 17:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

improvement The functionality exists but it could be improved in some way. tech debt rework due to choosing easy/limited solution Traffic Ops related to Traffic Ops

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants