On Tue, Oct 25, 2011 at 09:10:27PM +0200, Michael Hanselmann wrote:
> These two calls, “upload_file” and “write_ssconf_files” are treated
> separated as they're used by the configuration, where we can't use the
> normal resolver.
> 
> There's still some duplicated code in rpc.py, but that will be taken
> care of in future patches.

> diff --git a/lib/config.py b/lib/config.py
> index 268304f..7ca7c55 100644
> --- a/lib/config.py
> +++ b/lib/config.py
> @@ -1723,8 +1723,9 @@ class ConfigWriter:
>        node_list.append(node_info.name)
>        addr_list.append(node_info.primary_ip)
>  
> -    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.

>      for to_node, to_result in result.items():
>        msg = to_result.fail_msg
>        if msg:
> @@ -1783,7 +1784,7 @@ class ConfigWriter:
>      # Write ssconf files on all nodes (including locally)
>      if self._last_cluster_serial < self._config_data.cluster.serial_no:
>        if not self._offline:
> -        result = rpc.RpcRunner.call_write_ssconf_files(
> +        result = rpc.ConfigRunner(None).call_write_ssconf_files(
>            self._UnlockedGetOnlineNodeList(),
>            self._UnlockedGetSsconfValues())
>  
> diff --git a/lib/rpc.py b/lib/rpc.py
> index 1ecff15..8e235ed 100644
> --- a/lib/rpc.py
> +++ b/lib/rpc.py
> @@ -438,7 +438,8 @@ class _RpcProcessor:
>  
>  
>  class RpcRunner(_generated_rpc.RpcClientDefault,
> -                _generated_rpc.RpcClientBootstrap):
> +                _generated_rpc.RpcClientBootstrap,
> +                _generated_rpc.RpcClientConfig):
>    """RPC runner class.
>  
>    """
> @@ -453,6 +454,7 @@ class RpcRunner(_generated_rpc.RpcClientDefault,
>      # <http://www.logilab.org/ticket/36586> and
>      # <http://www.logilab.org/ticket/35642>
>      # pylint: disable=W0233
> +    _generated_rpc.RpcClientConfig.__init__(self)
>      _generated_rpc.RpcClientBootstrap.__init__(self)
>      _generated_rpc.RpcClientDefault.__init__(self)
>  
> @@ -639,47 +641,20 @@ class RpcRunner(_generated_rpc.RpcClientDefault,
>  
>      return ieioargs
>  
> -  #
> -  # Begin RPC calls
> -  #
> -
> -  @classmethod
> -  @_RpcTimeout(_TMO_NORMAL)
> -  def call_upload_file(cls, node_list, file_name, address_list=None):
> -    """Upload a file.
> -
> -    The node will refuse the operation in case the file is not on the
> -    approved file list.
> -
> -    This is a multi-node call.
> -
> -    @type node_list: list
> -    @param node_list: the list of node names to upload to
> -    @type file_name: str
> -    @param file_name: the filename to upload
> -    @type address_list: list or None
> -    @keyword address_list: an optional list of node addresses, in order
> -        to optimize the RPC speed
> +  @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.

thanks,
iustin

Reply via email to