On Wed, Oct 26, 2011 at 10:46:21AM +0200, Michael Hanselmann wrote:
> Am 26. Oktober 2011 10:34 schrieb Iustin Pop <[email protected]>:
> > On Tue, Oct 25, 2011 at 09:10:27PM +0200, Michael Hanselmann wrote:
> >> --- a/lib/config.py
> >> +++ b/lib/config.py
> >> -    result = rpc.RpcRunner.call_upload_file(node_list, self._cfg_file,
> >> -                                            address_list=addr_list)
> >> +    # TODO: Use dedicated resolver talking to config writer for name 
> >> resolution
> >> +    result = \
> >> +      rpc.ConfigRunner(addr_list).call_upload_file(node_list, 
> >> self._cfg_file)
> >
> > Hmm, we instantiate the ConfigRunner every time now.
> 
> Yes, but I'll write a custom resolver for the config writer which can
> access internal functions. Then this will not be necessary anymore. I
> just wanted to make it work for now.

Oh sure. It wasn't a "please change this", just a side comment.
> 
> >> --- a/lib/rpc.py
> >> +++ b/lib/rpc.py
> >> +  @staticmethod
> >> +  def _PrepareFileUpload(filename):
> >> +    """Loads a file and prepares it for an upload to nodes.
> >>
> >>      """
> >
> > Would it make sense to use the same strategy as in the previous patch
> > and move this to the caller? While it works, I realise now it's not the
> > nicest thing to do in a RPC library.
> 
> Yes, via a helper function. Let's do that in a separate patch again.

Ack.

I did say LGTM, right?

iustin

Reply via email to