-
Notifications
You must be signed in to change notification settings - Fork 40
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
Refactor bootstrap and run commands #109
Conversation
|
|
Needs a
|
|
Tested after removing existing avocado, works well. |
PowerVM:
|
PowerNV:
|
@sathnaga PowerNV package list needs handling when |
import sys | ||
import argparse | ||
import ConfigParser | ||
import binascii | ||
from shutil import copyfile | ||
from lib.logger import logger_init | ||
from lib.helper import runcmd | ||
from lib import helper |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can import helper
and use the same to invoke as helper.runcmd()
lib/helper.py
Outdated
:return: Status and output of the command | ||
""" | ||
if info_str: | ||
logger.info("%s", info_str) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logger.info(info_str)
lib/helper.py
Outdated
if info_str: | ||
logger.info("%s", info_str) | ||
if debug_str: | ||
logger.debug("%s", debug_str) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
lib/helper.py
Outdated
if err_str: | ||
logger.error("%s %s", err_str, output) | ||
sys.exit(1) | ||
logger.debug("%s", output) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
lib/helper.py
Outdated
return dist | ||
|
||
|
||
def get_dist_ver(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_dist()
and get_dist_ver()
can be merged to a single method.
cca6a69
to
1b0e428
Compare
@harish-24 @balamuruhans @narasimhan-v @PraveenPenguin needs your quick check and ack...lets get this in and do further improvements... |
@sles15 and rhel8.0 lpar would be more helpful, rest other envs aswell welcome... |
tested on fedora powernv
unit testing
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
SLES15SP1:
RHEL8
Both on PowerVM |
SLES 12 SP3 on PowerVM
|
RHEL 8 PowerNV:
|
avocado-setup.py
Outdated
logger.error("%s %s", err_str, error) | ||
sys.exit(1) | ||
helper.runcmd(cmd, | ||
debug_str="Cloning the repo: %s" % repo_name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sathnaga why is this debug_str
and not info_str
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, it could be Cloning the repo: %s in %s % (repo_name, repo_path)
, similar to the updating
section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@narasimhan-v makes sense making this change,
git diff
diff --git a/avocado-setup.py b/avocado-setup.py
index 33d21ee..aa887f0 100644
--- a/avocado-setup.py
+++ b/avocado-setup.py
@@ -204,7 +204,7 @@ def install_repo(path, name):
:param name: name of the repository
"""
cmd = "cd %s;make requirements;make requirements-selftests;python setup.py install" % path
- helper.runcmd(cmd, info_str="Installing repo: %s" % path,
+ helper.runcmd(cmd, info_str="Installing %s from %s" % (name, path),
err_str="Failed to install %s repository:" % name)
@@ -225,7 +225,7 @@ def get_repo(repo, basepath, install=False):
else:
cmd = "cd %s;git clone %s %s" % (basepath, repo, repo_name)
helper.runcmd(cmd,
- debug_str="Cloning the repo: %s" % repo_name,
+ info_str="Cloning the repo: %s in %s" % (repo_name, repo_path),
err_str="Failed to clone %s repository:" % repo_name)
if install:
install_repo(repo_path, repo_name)
1. Let's avoid bootstrapping kvm tests in powervm lpar and if asked explicilty. 2. Adds a wrapper for running commands and exit incase of failure. 3. Added independant package list based on environment, that way we can avoid unwanted package install. Signed-off-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Thanks all, merging now... |
Let's avoid bootstrapping kvm tests in powervm lpar
and if asked explicilty.
Adds a wrapper for running commands and exit incase
of failure.
Signed-off-by: Satheesh Rajendran sathnaga@linux.vnet.ibm.com