On Thu, Sep 03, 2015 at 10:36:49AM +0200, Jiri Prochazka wrote:
2015-09-02 15:18 GMT+02:00 Ondrej Lichtner olichtne@redhat.com:
On Fri, Aug 28, 2015 at 11:17:18AM +0200, Jiri Prochazka wrote:
This patch introduces several changes to the _query_pool_dir method in
lnst.Controller.Wizard:
- Method was renamed to _check_and_query_pool_dir to better express
nature of the method
- Method now uses 1 optional argument - pool_dir - if set to something
else than None,
it checks if it's valid path to existing or creatable dir, if it is,
uses it,
else queries user for a new path (which is also checked)
- Method noninteractive has added default value (None) for argument
pool_dir
This method was reworked to prevent code duplication which would occur
when mode for virtual
machines would be introduced
Signed-off-by: Jiri Prochazka jprochaz@redhat.com
lnst/Controller/Wizard.py | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-)
diff --git a/lnst/Controller/Wizard.py b/lnst/Controller/Wizard.py index 51856e8..b35b216 100644 --- a/lnst/Controller/Wizard.py +++ b/lnst/Controller/Wizard.py @@ -32,22 +32,7 @@ class Wizard: """ Starts Wizard in an interactive mode @param pool_dir Path to pool directory (optional) """
if pool_dir is None:
pool_dir = self._query_pool_dir()
else:
rv = self._check_path(pool_dir)
if rv == PATH_IS_DIR_ACCESSIBLE:
print("Pool directory set to '%s'" % pool_dir)
elif rv == PATH_DOES_NOT_EXIST:
sys.stderr.write("Path '%s' does not exist\n" %
pool_dir)
pool_dir = self._query_pool_dir()
elif rv == PATH_NOT_DIR:
sys.stderr.write("Path '%s' exists but is not a
directory\n"
% pool_dir)
pool_dir = self._query_pool_dir()
elif rv == PATH_IS_DIR_NOT_ACCESSIBLE:
sys.stderr.write("Directory '%s' is not writable\n" %
pool_dir)
pool_dir = self._query_pool_dir()
pool_dir = self._check_and_query_pool_dir(pool_dir) while True: hostname = self._query_hostname()
@@ -75,7 +60,7 @@ class Wizard: else: break
- def noninteractive(self, hostlist, pool_dir):
- def noninteractive(self, hostlist, pool_dir=None):
- Why "None" and not "DefaultPoolDir" ?
Because when pool_dir is set to None, the loop will continue and user is queried again.
This is noninteractive mode... no user queries.
- Doesn't it make more sense to split this into it's own patch? It
doesn't look like it's logically connected to anything else changed in this patch.
To split what in it's own patch?
This change of the default parameter, it doesn't look like it's connected to any other changes this patch makes so shouldn't it be in it's own patch?
""" Starts Wizard in noninteractive mode @param hostlist List of hosts (mandatory) @param pool_dir Path to pool_directory (optional)
@@ -330,13 +315,15 @@ class Wizard: sys.stderr.write("Hostname '%s' is not translatable
into a "
"valid IP address\n" % hostname)
- def _query_pool_dir(self):
- def _check_and_query_pool_dir(self, pool_dir): """ Queries user for pool directory
@param pool_dir Optional pool_dir which will be checked and
used if OK
@return Valid (is writable by user) path to directory """ while True:
pool_dir = raw_input("Enter path to a pool directory "
"(default: '%s'): " % DefaultPoolDir)
if pool_dir is None:
pool_dir = raw_input("Enter path to a pool directory "
"(default: '%s'): " %
DefaultPoolDir)
if pool_dir == "": pool_dir = DefaultPoolDir
@@ -359,6 +346,7 @@ class Wizard: elif rv == PATH_IS_DIR_NOT_ACCESSIBLE: sys.stderr.write("Directory '%s' is not writable\n" % pool_dir)
pool_dir = None
def _query_port(self): """ Queries user for port
-- 2.4.3
LNST-developers mailing list LNST-developers@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/lnst-developers