[Server-devel] [PATCH] Touch a ".transfer_complete" to mark completion, minor cleanups

[Michael Stone]

> -def new_backup_notify(server, nonce, xo_serial):
> -    try:
> -        auth = sha.sha(nonce + xo_serial)
> -        # TODO: add auth header
> -        ret = urllib.urlopen(server + '/new/%s' % xo_serial).read()
> -    except IOError, e:
> -        if e[1] == 403:
> -            # Auth not accepted. Shouldn't normally happen.
> -            raise BackupError(server)
> -

Why are we taking out this part of Ivan's protocol? As I recall, this
part of the protocol is important for mitigating DoS attacks on the
backup server...


>  def rsync_to_xs(from_path, to_path, keyfile, user):
>  
>      # add a trailing slash to ensure
> -    rsync = """/usr/bin/rsync -az --partial --delete --timeout=160 -e '%s' '%s' '%s' """ % \
> +    rsync = "/usr/bin/rsync -az --partial --delete --timeout=160 -e '%s' '%s' '%s' " % \
...
>      rsync_p = popen2.Popen3(rsync, True)
...
> -    rsync_exit = rsync_p.wait()
> +    rsync_exit = os.WEXITSTATUS(rsync_p.wait())
> +    if rsync_exit != 0:
> +        # TODO: retry a couple ofd times
> +        # if rsync_exit is 30 (Timeout in data send/receive)
> +        raise TransferError('rsync error code %s, message:'
> +                            % rsync_exit, rsync_p.childerr.read())


Since we're waiting for rsync to complete, is there any reason we aren't
using subprocess.check_call() to wrap all this up in a bow tie?



> +
> +    # Transfer an empty file marking completion
> +    # so the XS can see we are done.
> +    tmpfile = tempfile.mkstemp()

Any reason to leave the fd returned as the first component
tempfile.mkstemp() open?

Also, do we delete the tempfile when we're done with it?

> +    rsync = ("/usr/bin/rsync --timeout 10 -e '%s' '%s' '%s' "
> +             % (ssh, tmpfile[1], to_path+'/.transfer_complete'))
> +    rsync_p = popen2.Popen3(rsync, True)
> +    rsync_exit = os.WEXITSTATUS(rsync_p.wait())
>      if rsync_exit != 0:
> -        raise TransferError('rsync error code %s, message:' % rsync_exit, rsync_p.childerr.read())
> +        # TODO: retry a couple ofd times
> +        # if rsync_exit is 30 (Timeout in data send/receive)
> +        raise TransferError('rsync error code %s, message:'
> +                            % rsync_exit, rsync_p.childerr.read())
>  

Same comments about using subprocess.check_call().

Michael