Patchwork [2,of,7] bundle2: add a 'salvageoutput' method on bundle20

login
register
mail settings
Submitter Pierre-Yves David
Date April 16, 2015, 9:25 a.m.
Message ID <3e1f69627291b38adf39.1429176344@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/8704/
State Accepted
Headers show

Comments

Pierre-Yves David - April 16, 2015, 9:25 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1429168564 14400
#      Thu Apr 16 03:16:04 2015 -0400
# Node ID 3e1f69627291b38adf39a87a78cf48089f767a24
# Parent  878a6e0f8c834aec2cc8c50d65b9ab804644b73c
bundle2: add a 'salvageoutput' method on bundle20

This method returns a copy of every 'output' parts added to the bundler.

This is the second step in our quest for preserving the server output on error
(issue4594). We want to be able to copy the output parts from the aborted reply
into the exception bundle.

The function will be used in a later patch.
Ryan McElroy - April 16, 2015, 6:17 p.m.
On 4/16/2015 2:25 AM, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1429168564 14400
> #      Thu Apr 16 03:16:04 2015 -0400
> # Node ID 3e1f69627291b38adf39a87a78cf48089f767a24
> # Parent  878a6e0f8c834aec2cc8c50d65b9ab804644b73c
> bundle2: add a 'salvageoutput' method on bundle20

s/bundle20/bundle2

> This method returns a copy of every 'output' parts added to the bundler.

enlgish nit-pick: s/every/all

>
> This is the second step in our quest for preserving the server output on error
> (issue4594). We want to be able to copy the output parts from the aborted reply
> into the exception bundle.
>
> The function will be used in a later patch.
>
> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
> --- a/mercurial/bundle2.py
> +++ b/mercurial/bundle2.py
> @@ -478,10 +478,22 @@ class bundle20(object):
>                   value = urllib.quote(value)
>                   par = '%s=%s' % (par, value)
>               blocks.append(par)
>           return ' '.join(blocks)
>   
> +    def salvageoutput(self):
> +        """return of list with a copy of all output parts in the bundle
> +
> +        This is meant to be used during error handling to make sure we preserve
> +        server output"""
> +        salvaged = []
> +        for part in self._parts:
> +            if part.type.startswith('output'):

This seems fragile. I'd prefer a method like `part.isoutput()`

> +                salvaged.append(part.copy())
> +        return salvaged
> +
> +
>   class unpackermixin(object):
>       """A mixin to extract bytes and struct data from a stream"""
>   
>       def __init__(self, fp):
>           self._fp = fp
>
Pierre-Yves David - April 16, 2015, 7:38 p.m.
On 04/16/2015 02:17 PM, Ryan McElroy wrote:
> On 4/16/2015 2:25 AM, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>> # Date 1429168564 14400
>> #      Thu Apr 16 03:16:04 2015 -0400
>> # Node ID 3e1f69627291b38adf39a87a78cf48089f767a24
>> # Parent  878a6e0f8c834aec2cc8c50d65b9ab804644b73c
>> bundle2: add a 'salvageoutput' method on bundle20
>
> s/bundle20/bundle2

Nope I mean bundle20, the class that generate 'HG20' file

>> This method returns a copy of every 'output' parts added to the bundler.
>
> english nit-pick: s/every/all

Sure, maybe matt can fix this inflight?

>> This is the second step in our quest for preserving the server output
>> on error
>> (issue4594). We want to be able to copy the output parts from the
>> aborted reply
>> into the exception bundle.
>>
>> The function will be used in a later patch.
>>
>> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
>> --- a/mercurial/bundle2.py
>> +++ b/mercurial/bundle2.py
>> @@ -478,10 +478,22 @@ class bundle20(object):
>>                   value = urllib.quote(value)
>>                   par = '%s=%s' % (par, value)
>>               blocks.append(par)
>>           return ' '.join(blocks)
>> +    def salvageoutput(self):
>> +        """return of list with a copy of all output parts in the bundle
>> +
>> +        This is meant to be used during error handling to make sure
>> we preserve
>> +        server output"""
>> +        salvaged = []
>> +        for part in self._parts:
>> +            if part.type.startswith('output'):
>
> This seems fragile. I'd prefer a method like `part.isoutput()`

Meh. We probably want some more robust and wide salvaging. But the 
freeze day is now so I rather get this simple version in to have bundle2 
usable in 3.4. We can improve this salvaging mechanism later.

Patch

diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -478,10 +478,22 @@  class bundle20(object):
                 value = urllib.quote(value)
                 par = '%s=%s' % (par, value)
             blocks.append(par)
         return ' '.join(blocks)
 
+    def salvageoutput(self):
+        """return of list with a copy of all output parts in the bundle
+
+        This is meant to be used during error handling to make sure we preserve
+        server output"""
+        salvaged = []
+        for part in self._parts:
+            if part.type.startswith('output'):
+                salvaged.append(part.copy())
+        return salvaged
+
+
 class unpackermixin(object):
     """A mixin to extract bytes and struct data from a stream"""
 
     def __init__(self, fp):
         self._fp = fp