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

[Ondrej Lichtner]

On Thu, Mar 17, 2016 at 11:49:26AM +0100, Jan Tluka wrote:
>  Wed, Mar 02, 2016 at 02:46:15PM CET, jprochaz(a)redhat.com 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
> lnst-developers(a)lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/[e-mail]...