[PATCH 1/3] lnst-ctl: add list_pools action

[Jiri Prochazka]

This patch adds new action called list_pools. It has two modes of
execution, non-verbose and verbose. Non verbose, executed just with
"lnst-ctl list_pools" command, prints out pool names and their paths
in pool_name=/path/to/pool format.

For verbose mode, option -v or --verbose must be entered. It prints
pool names and their paths and in addition, it lists through all
.xml files in pool directories and prints their contents in more
human readable format.

Pools are acquired from all the configs lnst-ctl would use in
normal run.

Signed-off-by: Jiri Prochazka 
---
 lnst-ctl | 63 +++++++++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 49 insertions(+), 14 deletions(-)

diff --git a/lnst-ctl b/lnst-ctl
index 6d656f8..5a32f7c 100755
--- a/lnst-ctl
+++ b/lnst-ctl
@@ -24,6 +24,8 @@ from lnst.Common.Utils import mkdir_p
 from lnst.Controller.NetTestController import NetTestController, NetTestError
 from lnst.Controller.NetTestController import NoMatchError
 from lnst.Controller.NetTestResultSerializer import NetTestResultSerializer
+from lnst.Controller.SlaveMachineParser import SlaveMachineParser
+from lnst.Controller.SlavePool import SlavePool
 from lnst.Controller.XmlProcessing import XmlProcessingError

RETVAL_PASS = 0
@@ -35,7 +37,8 @@ def usage(retval=0):
     """
     print "Usage: %s [OPTIONS...] ACTION [RECIPES...]" % sys.argv[0]
     print ""
-    print "ACTION = [ run | config_only | deconfigure | match_setup ]"
+    print "ACTION = [ run | config_only | deconfigure | match_setup | " \
+                     "list_pools]"
     print ""
     print "OPTIONS"
     print "  -A, --override-alias name=value define top-level alias that " \
@@ -45,8 +48,8 @@ def usage(retval=0):
     print "  -d, --debug                     emit debugging messages"
     print "      --dump-config               dumps the join of all loaded " \
               "configuration files on stdout and exits"
-    print "      --dump-pools                dumps the available machine " \
-              "pools and exits"
+    print "  -v, --verbose                   verbose version of list_pools " \
+              "command"
     print "  -h, --help                      print this message"
     print "  -m, --no-colours                disable coloured terminal output"
     print "  -o, --disable-pool-checks       don't check the availability of " \
@@ -67,6 +70,35 @@ def usage(retval=0):
     print "  -x, --result=FILE               file to write xml_result"
     sys.exit(retval)

+def list_pools():
+    # iterate over all pools
+    for pool_name, pool_path in lnst_config.get_pools().items():
+        print "%s (%s)" % (pool_name, pool_path)
+        dentries = os.listdir(pool_path)
+        # iterate over all slave machine cfgs
+        for dirent in dentries:
+            filepath = pool_path + "/" + dirent
+            dirname, basename = os.path.split(filepath)
+            if os.path.isfile(filepath) and re.search(".xml$", filepath, re.I):
+                parser = SlaveMachineParser(filepath)
+                xml_data = parser.parse()
+                # parse XML to dict
+                res = SlavePool._process_machine_xml_data(SlavePool({}), "", xml_data)
+                # print in human-readable format
+                print " ", "Filename: ", basename
+                if res['params']:
+                    print 3*" ", "Parameters:"
+                for key in res['params']:
+                    print 7*" ", key+":", res['params'][key]
+                if res['interfaces']:
+                    print 3*" ", "Interfaces:"
+                for key in res['interfaces']:
+                    print 5*" ", "id:", key, "\tnetwork:", res['interfaces'][key]['network']
+                    for param in res['interfaces'][key]['params']:
+                        print 7*" ", param+":", res['interfaces'][key]['params'][param]
+                print ""
+    return RETVAL_PASS
+
 def store_alias(alias_def, aliases_dict):
     try:
         name, value = alias_def.split("=")
@@ -189,7 +221,7 @@ def main():
     try:
         opts, args = getopt.getopt(
             sys.argv[1:],
-            "A:a:c:dhmoprs:t:ux:",
+            "A:a:c:dhmoprs:t:ux:v",
             [
              "override_alias=",
              "define_alias=",
@@ -206,7 +238,7 @@ def main():
              "multi-match",
              "result=",
              "pools=",
-             "dump-pools"
+             "verbose"
             ]
         )
     except getopt.GetoptError as err:
@@ -246,7 +278,7 @@ def main():
     reduce_sync = False
     multi_match = False
     dump_config = False
-    dump_pools = False
+    verbose = False
     pools = []
     for opt, arg in opts:
         if opt in ("-d", "--debug"):
@@ -283,8 +315,8 @@ def main():
             dump_config = True
         elif opt in ("--pools"):
             pools.extend(arg.split(","))
-        elif opt in ("--dump-pools"):
-            dump_pools = True
+        elif opt in ("-v", "--verbose"):
+            verbose= True

if xslt_url != None:
         lnst_config.set_option("environment", "xslt_url", xslt_url)
@@ -293,11 +325,6 @@ def main():
         print lnst_config.dump_config()
         return RETVAL_PASS

-    if dump_pools:
-        for pool_name, pool_path in lnst_config.get_pools().items():
-            print pool_name + ' = ' + pool_path
-        return RETVAL_PASS
-
     if coloured_output:
         coloured_output = not lnst_config.get_option("colours",
                                                      "disable_colours")
@@ -315,12 +342,20 @@ def main():

action = args[0]
     recipes = args[1:]
-    if not action in ['run', 'config_only', 'deconfigure', 'match_setup']:
+    if not action in ['run', 'config_only', 'deconfigure', 'match_setup',
+                      'list_pools']:
         logging.error("Action '%s' not recognised" % action)
         usage(RETVAL_ERR)
     if action == 'deconfigure':
         NetTestController.remove_saved_machine_config()
         return 0
+    elif action == 'list_pools':
+        logging.disable(logging.CRITICAL)
+        for pool_name, pool_path in lnst_config.get_pools().items():
+            print pool_name + ' = ' + pool_path
+        if verbose:
+            list_pools()
+        return RETVAL_PASS

if recipes == []:
         logging.error("No recipe specified!")

[PATCH 2/3] lnst-ctl: add bash autocompletion for list_pools action

[Jiri Prochazka]

updates bash autocompletion file with list_pools action and
verbose option

Signed-off-by: Jiri Prochazka 
---
 install/lnst-ctl.bash | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/install/lnst-ctl.bash b/install/lnst-ctl.bash
index 05a9b6d..62aab66 100644
--- a/install/lnst-ctl.bash
+++ b/install/lnst-ctl.bash
@@ -14,15 +14,15 @@ _list_has_item()

_lnst_ctl()
 {
-    local SHORT_OPTS="-a -A -c -d -h -m -o -p -x"
+    local SHORT_OPTS="-a -A -c -d -h -m -o -p -x -v"
     local LONG_OPTS="--define-alias --override-alias --config --debug \
                      --help --no-colours --disable-pool-checks \
-                     --packet-capture --result"
+                     --packet-capture --result --verbose"
     local REQUIRE_ARG="-a --define-alias \
                        -A --override-alias \
                        -c --config \
                        -x --result"
-    local ACTIONS="config_only match_setup run"
+    local ACTIONS="config_only match_setup run list_pools"

local cur=${COMP_WORDS[COMP_CWORD]}
     local prev=${COMP_WORDS[COMP_CWORD-1]}

[PATCH 3/3] lnst-ctl: update man pages for list_pools action

[Jiri Prochazka]

updates lnst-ctl man pages, removing --dump-pools option and
adding list_pools action along with verbose option

Signed-off-by: Jiri Prochazka 
---
 install/lnst-ctl.1.in | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/install/lnst-ctl.1.in b/install/lnst-ctl.1.in
index 77d9bb8..28c80b6 100644
--- a/install/lnst-ctl.1.in
+++ b/install/lnst-ctl.1.in
@@ -45,9 +45,6 @@ Toggle emitting debugging messages.
 .B --dump-config
 Dumps the join of all loaded configuration files on stdout and exits
 .TP
-.B --dump-pools
-Dumps the available machine pools and exits
-.TP
 .B -h, --help
 Display usage of this command.
 .TP
@@ -86,6 +83,9 @@ Run each recipe with every pool match possible
 .BI "-x, --result" =file
 Write results in XML format to the specified
 .IR file .
+.TP
+.B -v, --verbose
+Extends output of list_pools action
 .PP
 .I RECIPE
 can be either a LNST recipe file or a directory containing recipe files.
@@ -99,8 +99,9 @@ bellow).
 can be one of
 .BR "run",
 .BR "config_only",
-.BR "deconfigure" ", and"
-.BR "match_setup".
+.BR "deconfigure" ",
+.BR "match_setup",\ and
+.BR "list_pools".

When it is set to
 .BR "run",
@@ -122,6 +123,17 @@ the configuration from a previous config_only run will be removed, including
 dynamically created devices on virtual machines. There is no need to specify a
 recipe for this action.

+In case of
+.BR "list_pools",
+lnst-ctl looks up all the pools entered in configs and dumps info about them.
+When used without
+.B -v
+or
+.B --verbose
+option, only pool names and paths are printed.
+With verbose option, all viable slave machine configs are printed in more
+human readable format.
+
 At last, when the
 .I ACTION
 is set to

[Jan Tluka]

Wed, Mar 02, 2016 at 12:06:31PM CET, [e-mail] wrote:
> This patch adds new action called list_pools. It has two modes of
> execution, non-verbose and verbose. Non verbose, executed just with
> "lnst-ctl list_pools" command, prints out pool names and their paths
> in pool_name=/path/to/pool format.
>
> For verbose mode, option -v or --verbose must be entered. It prints
> pool names and their paths and in addition, it lists through all
> .xml files in pool directories and prints their contents in more
> human readable format.
>
> Pools are acquired from all the configs lnst-ctl would use in
> normal run.

I think you should also include that you're substituting the dump-pools
command, right? It's quite big change from the user point of view so
don't forget to include this in the commit message.
> Signed-off-by: Jiri Prochazka
>
> lnst-ctl | 63 +++++++++++++++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 49 insertions(+), 14 deletions(-)
>
> diff --git a/lnst-ctl b/lnst-ctl
> index 6d656f8..5a32f7c 100755
> --- a/lnst-ctl
> +++ b/lnst-ctl
> @@ -24,6 +24,8 @@ from lnst.Common.Utils import mkdir_p
> from lnst.Controller.NetTestController import NetTestController, NetTestError
> from lnst.Controller.NetTestController import NoMatchError
> from lnst.Controller.NetTestResultSerializer import NetTestResultSerializer
> +from lnst.Controller.SlaveMachineParser import SlaveMachineParser
> +from lnst.Controller.SlavePool import SlavePool
> from lnst.Controller.XmlProcessing import XmlProcessingError
>
> RETVAL_PASS = 0
> @@ -35,7 +37,8 @@ def usage(retval=0):
>     """
>     print "Usage: %s [OPTIONS...] ACTION [RECIPES...]" % sys.argv[0]
>     print ""
> print "ACTION = [ run | config_only | deconfigure | match_setup ]"
> print "ACTION = [ run | config_only | deconfigure | match_setup | " \
>                 "list_pools]"
> print ""
>   print "OPTIONS"
>   print "  -A, --override-alias name=value define top-level alias that " \
>
> @@ -45,8 +48,8 @@ def usage(retval=0):
>     print "  -d, --debug                     emit debugging messages"
>     print "      --dump-config               dumps the join of all loaded " \
>               "configuration files on stdout and exits"
> print "      --dump-pools                dumps the available machine " \
>          "pools and exits"
> print "  -v, --verbose                   verbose version of list_pools " \
>          "command"
> print "  -h, --help                      print this message"
>   print "  -m, --no-colours                disable coloured terminal output"
>   print "  -o, --disable-pool-checks       don't check the availability of " \
>
> @@ -67,6 +70,35 @@ def usage(retval=0):
>     print "  -x, --result=FILE               file to write xml_result"
>     sys.exit(retval)
>
> +def list_pools():
> # iterate over all pools
> for pool_name, pool_path in lnst_config.get_pools().items():
>    print "%s (%s)" % (pool_name, pool_path)
>    dentries = os.listdir(pool_path)
>    # iterate over all slave machine cfgs
>    for dirent in dentries:
>        filepath = pool_path + "/" + dirent
>        dirname, basename = os.path.split(filepath)
>        if os.path.isfile(filepath) and re.search("\.xml$", filepath, re.I):
>            parser = SlaveMachineParser(filepath)
>            xml_data = parser.parse()
>            # parse XML to dict
>            res = SlavePool._process_machine_xml_data(SlavePool({}), "", xml_data)

The underscore means that this method should not be called directly.
>            # print in human-readable format
>            print " ", "Filename: ", basename
>            if res['params']:
>                print 3*" ", "Parameters:"
>            for key in res['params']:
>                print 7*" ", key+":", res['params'][key]
>            if res['interfaces']:
>                print 3*" ", "Interfaces:"
>            for key in res['interfaces']:
>                print 5*" ", "id:", key, "\tnetwork:", res['interfaces'][key]['network']
>                for param in res['interfaces'][key]['params']:
>                    print 7*" ", param+":", res['interfaces'][key]['params'][param]
>            print ""
> return RETVAL_PASS
>
> def store_alias(alias_def, aliases_dict):
>     try:
>         name, value = alias_def.split("=")
> @@ -189,7 +221,7 @@ def main():
>     try:
>         opts, args = getopt.getopt(
>             sys.argv[1:],
>        "A:a:c:dhmoprs:t:ux:",
>        "A:a:c:dhmoprs:t:ux:v",
>       [
>        "override_alias=",
>        "define_alias=",
>
> @@ -206,7 +238,7 @@ def main():
>              "multi-match",
>              "result=",
>              "pools=",
>         "dump-pools"
>         "verbose"
>       ]
>   )
> except getopt.GetoptError as err:
>
> @@ -246,7 +278,7 @@ def main():
>     reduce_sync = False
>     multi_match = False
>     dump_config = False
> dump_pools = False
> verbose = False
> pools = []
> for opt, arg in opts:
>    if opt in ("-d", "--debug"):
>
> @@ -283,8 +315,8 @@ def main():
>             dump_config = True
>         elif opt in ("--pools"):
>             pools.extend(arg.split(","))
>    elif opt in ("--dump-pools"):
>        dump_pools = True
>    elif opt in ("-v", "--verbose"):
>        verbose= True
>
> if xslt_url != None:
>       lnst_config.set_option("environment", "xslt_url", xslt_url)
>
> @@ -293,11 +325,6 @@ def main():
>         print lnst_config.dump_config()
>         return RETVAL_PASS
> if dump_pools:
>    for pool_name, pool_path in lnst_config.get_pools().items():
>        print pool_name + ' = ' + pool_path
>    return RETVAL_PASS
> if coloured_output:
>       coloured_output = not lnst_config.get_option("colours",
>                                                    "disable_colours")
>
> @@ -315,12 +342,20 @@ def main():
> action = args[0]
> recipes = args[1:]
> if not action in ['run', 'config_only', 'deconfigure', 'match_setup']:
>
> if not action in ['run', 'config_only', 'deconfigure', 'match_setup',
>                  'list_pools']:
>   logging.error("Action '%s' not recognised" % action)
>   usage(RETVAL_ERR)
>
> if action == 'deconfigure':
>       NetTestController.remove_saved_machine_config()
>       return 0
>
> elif action == 'list_pools':
>    logging.disable(logging.CRITICAL)
>    for pool_name, pool_path in lnst_config.get_pools().items():
>        print pool_name + ' = ' + pool_path
>    if verbose:
>        list_pools()
>    return RETVAL_PASS
>
> if recipes == []:
>       logging.error("No recipe specified!")
>
> -- 
> 2.4.3
> _______________________________________________
> LNST-developers mailing list
> [e-mail]
> https://lists.fedorahosted.org/admin/lists/[e-mail]...

[Jiri Prochazka]

2016-03-02 12:56 GMT+01:00 Jan Tluka :
> Wed, Mar 02, 2016 at 12:06:31PM CET, [e-mail] wrote:
> > This patch adds new action called list_pools. It has two modes of
> > execution, non-verbose and verbose. Non verbose, executed just with
> > "lnst-ctl list_pools" command, prints out pool names and their paths
> > in pool_name=/path/to/pool format.
> >
> > For verbose mode, option -v or --verbose must be entered. It prints
> > pool names and their paths and in addition, it lists through all
> > .xml files in pool directories and prints their contents in more
> > human readable format.
> >
> > Pools are acquired from all the configs lnst-ctl would use in
> > normal run.
>
> I think you should also include that you're substituting the dump-pools
> command, right? It's quite big change from the user point of view so
> don't forget to include this in the commit message.

​ok, will do in v2 of the patchset, thanks​
> > Signed-off-by: Jiri Prochazka
> >
> > lnst-ctl | 63
>
> +++++++++++++++++++++++++++++++++++++++++++++++++--------------
> > 1 file changed, 49 insertions(+), 14 deletions(-)
> >
> > diff --git a/lnst-ctl b/lnst-ctl
> > index 6d656f8..5a32f7c 100755
> > --- a/lnst-ctl
> > +++ b/lnst-ctl
> > @@ -24,6 +24,8 @@ from lnst.Common.Utils import mkdir_p
> > from lnst.Controller.NetTestController import NetTestController,
>
> NetTestError
> > from lnst.Controller.NetTestController import NoMatchError
> > from lnst.Controller.NetTestResultSerializer import
>
> NetTestResultSerializer
> > +from lnst.Controller.SlaveMachineParser import SlaveMachineParser
> > +from lnst.Controller.SlavePool import SlavePool
> > from lnst.Controller.XmlProcessing import XmlProcessingError
> >
> > RETVAL_PASS = 0
> > @@ -35,7 +37,8 @@ def usage(retval=0):
> >     """
> >     print "Usage: %s [OPTIONS...] ACTION [RECIPES...]" % sys.argv[0]
> >     print ""
> > print "ACTION = [ run | config_only | deconfigure | match_setup ]"
> > print "ACTION = [ run | config_only | deconfigure | match_setup | " \
> >                 "list_pools]"
> > print ""
> >   print "OPTIONS"
> >   print "  -A, --override-alias name=value define top-level alias that
>
> " \
> > @@ -45,8 +48,8 @@ def usage(retval=0):
> >     print "  -d, --debug                     emit debugging messages"
> >     print "      --dump-config               dumps the join of all
>
> loaded " \
> >           "configuration files on stdout and exits"
> > print "      --dump-pools                dumps the available machine
>
> " \
> >          "pools and exits"
> > print "  -v, --verbose                   verbose version of
>
> list_pools " \
> >          "command"
> > print "  -h, --help                      print this message"
> >   print "  -m, --no-colours                disable coloured terminal
>
> output"
> > print "  -o, --disable-pool-checks       don't check the
>
> availability of " \
> > @@ -67,6 +70,35 @@ def usage(retval=0):
> >     print "  -x, --result=FILE               file to write xml_result"
> >     sys.exit(retval)
> >
> > +def list_pools():
> > # iterate over all pools
> > for pool_name, pool_path in lnst_config.get_pools().items():
> >    print "%s (%s)" % (pool_name, pool_path)
> >    dentries = os.listdir(pool_path)
> >    # iterate over all slave machine cfgs
> >    for dirent in dentries:
> >        filepath = pool_path + "/" + dirent
> >        dirname, basename = os.path.split(filepath)
> >        if os.path.isfile(filepath) and re.search("\.xml$",
>
> filepath, re.I):
> >            parser = SlaveMachineParser(filepath)
> >            xml_data = parser.parse()
> >            # parse XML to dict
> >            res = SlavePool._process_machine_xml_data(SlavePool({}),
>
> "", xml_data)
>
> The underscore means that this method should not be called directly.

​I know, I didn't want to write any duplicate code though. What would
you recommend as suggested approach to this? Rename and rework the method
or just write my own xml processing? Again, thanks for the feedback​
> >            # print in human-readable format
> >            print " ", "Filename: ", basename
> >            if res['params']:
> >                print 3*" ", "Parameters:"
> >            for key in res['params']:
> >                print 7*" ", key+":", res['params'][key]
> >            if res['interfaces']:
> >                print 3*" ", "Interfaces:"
> >            for key in res['interfaces']:
> >                print 5*" ", "id:", key, "\tnetwork:",
>
> res['interfaces'][key]['network']
> >                for param in res['interfaces'][key]['params']:
> >                    print 7*" ", param+":",
>
> res['interfaces'][key]['params'][param]
> >            print ""
> > return RETVAL_PASS
> >
> > def store_alias(alias_def, aliases_dict):
> >     try:
> >         name, value = alias_def.split("=")
> > @@ -189,7 +221,7 @@ def main():
> >     try:
> >         opts, args = getopt.getopt(
> >             sys.argv[1:],
> >        "A:a:c:dhmoprs:t:ux:",
> >        "A:a:c:dhmoprs:t:ux:v",
> >       [
> >        "override_alias=",
> >        "define_alias=",
> >
> > @@ -206,7 +238,7 @@ def main():
> >              "multi-match",
> >              "result=",
> >              "pools=",
> >         "dump-pools"
> >         "verbose"
> >       ]
> >   )
> > except getopt.GetoptError as err:
> >
> > @@ -246,7 +278,7 @@ def main():
> >     reduce_sync = False
> >     multi_match = False
> >     dump_config = False
> > dump_pools = False
> > verbose = False
> > pools = []
> > for opt, arg in opts:
> >    if opt in ("-d", "--debug"):
> >
> > @@ -283,8 +315,8 @@ def main():
> >             dump_config = True
> >         elif opt in ("--pools"):
> >             pools.extend(arg.split(","))
> >    elif opt in ("--dump-pools"):
> >        dump_pools = True
> >    elif opt in ("-v", "--verbose"):
> >        verbose= True
> >
> > if xslt_url != None:
> >       lnst_config.set_option("environment", "xslt_url", xslt_url)
> >
> > @@ -293,11 +325,6 @@ def main():
> >         print lnst_config.dump_config()
> >         return RETVAL_PASS
> > if dump_pools:
> >    for pool_name, pool_path in lnst_config.get_pools().items():
> >        print pool_name + ' = ' + pool_path
> >    return RETVAL_PASS
> > if coloured_output:
> >       coloured_output = not lnst_config.get_option("colours",
> >                                                    "disable_colours")
> >
> > @@ -315,12 +342,20 @@ def main():
> > action = args[0]
> > recipes = args[1:]
> > if not action in ['run', 'config_only', 'deconfigure',
>
> 'match_setup']:
> > if not action in ['run', 'config_only', 'deconfigure', 'match_setup',
> >                  'list_pools']:
> >   logging.error("Action '%s' not recognised" % action)
> >   usage(RETVAL_ERR)
> >
> > if action == 'deconfigure':
> >       NetTestController.remove_saved_machine_config()
> >       return 0
> >
> > elif action == 'list_pools':
> >    logging.disable(logging.CRITICAL)
> >    for pool_name, pool_path in lnst_config.get_pools().items():
> >        print pool_name + ' = ' + pool_path
> >    if verbose:
> >        list_pools()
> >    return RETVAL_PASS
> >
> > if recipes == []:
> >       logging.error("No recipe specified!")
> >
> > --
> > 2.4.3
> > _______________________________________________
> > LNST-developers mailing list
> > [e-mail]
>
> https://lists.fedorahosted.org/admin/lists/[e-mail]...

[Jan Tluka]

Wed, Mar 02, 2016 at 02:46:15PM CET, [e-mail] wrote:
> > >            parser = SlaveMachineParser(filepath)
> > >            xml_data = parser.parse()
> > >            # parse XML to dict
> > >            res = SlavePool._process_machine_xml_data(SlavePool({}),
> >
> > "", xml_data)
> >
> > The underscore means that this method should not be called directly.
>
> I know, I didn't want to write any duplicate code though. What would
> you recommend as suggested approach to this? Rename and rework the method
> or just write my own xml processing? Again, thanks for the feedback

Had a look at this again and IMO the two methods:
    _process_machine_xml_data
    _process_iface_xml_data

should be moved to SlaveMachineParser module, made public  and imported from
there.

I'd like to have Ondrej's view on this. He's currently on PTO, so this
has to wait a but longer.

-Jan

[Jan Tluka]

Thu, Mar 17, 2016 at 11:49:26AM CET, [e-mail] wrote:
> Wed, Mar 02, 2016 at 02:46:15PM CET, [e-mail] wrote:
> > > >            parser = SlaveMachineParser(filepath)
> > > >            xml_data = parser.parse()
> > > >            # parse XML to dict
> > > >            res = SlavePool._process_machine_xml_data(SlavePool({}),
> > >
> > > "", xml_data)
> > >
> > > The underscore means that this method should not be called directly.
> >
> > I know, I didn't want to write any duplicate code though. What would
> > you recommend as suggested approach to this? Rename and rework the method
> > or just write my own xml processing? Again, thanks for the feedback
>
> Had a look at this again and IMO the two methods:
>    _process_machine_xml_data
>    _process_iface_xml_data
>
> should be moved to SlaveMachineParser module, made public  and imported from
> there.

By that I mean they should not be part of any class but rather a standalone
functions.
> I'd like to have Ondrej's view on this. He's currently on PTO, so this
> has to wait a but longer.
>
> -Jan
> _______________________________________________
> LNST-developers mailing list
> [e-mail]
> https://lists.fedorahosted.org/admin/lists/[e-mail]...

[Ondrej Lichtner]

On Thu, Mar 17, 2016 at 11:49:26AM +0100, Jan Tluka wrote:
> Wed, Mar 02, 2016 at 02:46:15PM CET, [e-mail] wrote:
> > > >            parser = SlaveMachineParser(filepath)
> > > >            xml_data = parser.parse()
> > > >            # parse XML to dict
> > > >            res = SlavePool._process_machine_xml_data(SlavePool({}),
> > >
> > > "", xml_data)
> > >
> > > The underscore means that this method should not be called directly.
> >
> > I know, I didn't want to write any duplicate code though. What would
> > you recommend as suggested approach to this? Rename and rework the method
> > or just write my own xml processing? Again, thanks for the feedback
>
> Had a look at this again and IMO the two methods:
>     _process_machine_xml_data
>     _process_iface_xml_data
>
> should be moved to SlaveMachineParser module, made public  and imported from
> there.
>
> I'd like to have Ondrej's view on this. He's currently on PTO, so this
> has to wait a but longer.

Well... why not just create a SlavePool object (with machine checks
disabled) and then use it's _pools dictionary (add a get method for it
of course)? Would that be easier to do from lnst-ctl and also prettier?

-Ondrej
> -Jan
> _______________________________________________
> LNST-developers mailing list
> [e-mail]
> https://lists.fedorahosted.org/admin/lists/[e-mail]...

[Jan Tluka]

Mon, Mar 21, 2016 at 10:27:01AM CET, [e-mail] wrote:
> On Thu, Mar 17, 2016 at 11:49:26AM +0100, Jan Tluka wrote:
> > Wed, Mar 02, 2016 at 02:46:15PM CET, [e-mail] wrote:
> > > > >            parser = SlaveMachineParser(filepath)
> > > > >            xml_data = parser.parse()
> > > > >            # parse XML to dict
> > > > >            res = SlavePool._process_machine_xml_data(SlavePool({}),
> > > >
> > > > "", xml_data)
> > > >
> > > > The underscore means that this method should not be called directly.
> > >
> > > I know, I didn't want to write any duplicate code though. What would
> > > you recommend as suggested approach to this? Rename and rework the method
> > > or just write my own xml processing? Again, thanks for the feedback
> >
> > Had a look at this again and IMO the two methods:
> >     _process_machine_xml_data
> >     _process_iface_xml_data
> >
> > should be moved to SlaveMachineParser module, made public  and imported from
> > there.
> >
> > I'd like to have Ondrej's view on this. He's currently on PTO, so this
> > has to wait a but longer.
>
> Well... why not just create a SlavePool object (with machine checks
> disabled) and then use it's _pools dictionary (add a get method for it
> of course)? Would that be easier to do from lnst-ctl and also prettier?
>
> -Ondrej

Makes sense, I must have missed that.

-Jan
> > -Jan
> > _______________________________________________
> > LNST-developers mailing list
> > [e-mail]
> > https://lists.fedorahosted.org/admin/lists/[e-mail]...