Patchwork [1,of,7] bundle2: add a 'copy' method on part

login
register
mail settings
Submitter Pierre-Yves David
Date April 16, 2015, 9:25 a.m.
Message ID <878a6e0f8c834aec2cc8.1429176343@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/8703/
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 1429168528 14400
#      Thu Apr 16 03:15:28 2015 -0400
# Node ID 878a6e0f8c834aec2cc8c50d65b9ab804644b73c
# Parent  d5711c886d0b1acb2d18b92bf6e8ba6d9ad0c4b3
bundle2: add a 'copy' method on part

This is the first 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 dunction will be used in a later patch.
Ryan McElroy - April 16, 2015, 6:15 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 1429168528 14400
> #      Thu Apr 16 03:15:28 2015 -0400
> # Node ID 878a6e0f8c834aec2cc8c50d65b9ab804644b73c
> # Parent  d5711c886d0b1acb2d18b92bf6e8ba6d9ad0c4b3
> bundle2: add a 'copy' method on part
>
> This is the first 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 dunction will be used in a later patch.

s/The dunction/This function/g

>
> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
> --- a/mercurial/bundle2.py
> +++ b/mercurial/bundle2.py
> @@ -653,10 +653,19 @@ class bundlepart(object):
>           # - False: currently generated,
>           # - True: generation done.
>           self._generated = None
>           self.mandatory = mandatory
>   
> +    def copy(self):
> +        """return a copy of the part
> +
> +        The new part have the very same content but no partid assigned yet.
> +        Parts with generated data cannot be copied."""
> +        assert not util.safehasattr(self.data, 'next')

It is nonobvious to me that 'next' refers to generated data.

Also, I'd like this assert to have more debug information in it.

> +        return self.__class__(self.type, self._mandatoryparams,
> +                              self._advisoryparams, self._data, self.mandatory)
> +
>       # methods used to defines the part content
>       def __setdata(self, data):
>           if self._generated is not None:
>               raise error.ReadOnlyPartError('part is being generated')
>           self._data = data
>
Pierre-Yves David - April 16, 2015, 7:34 p.m.
On 04/16/2015 02:15 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 1429168528 14400
>> #      Thu Apr 16 03:15:28 2015 -0400
>> # Node ID 878a6e0f8c834aec2cc8c50d65b9ab804644b73c
>> # Parent  d5711c886d0b1acb2d18b92bf6e8ba6d9ad0c4b3
>> bundle2: add a 'copy' method on part
>>
>> This is the first 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 dunction will be used in a later patch.
>
> s/The dunction/This function/g

Sure, maybe matt can fix that inflight?

>> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
>> --- a/mercurial/bundle2.py
>> +++ b/mercurial/bundle2.py
>> @@ -653,10 +653,19 @@ class bundlepart(object):
>>           # - False: currently generated,
>>           # - True: generation done.
>>           self._generated = None
>>           self.mandatory = mandatory
>> +    def copy(self):
>> +        """return a copy of the part
>> +
>> +        The new part have the very same content but no partid
>> assigned yet.
>> +        Parts with generated data cannot be copied."""
>> +        assert not util.safehasattr(self.data, 'next')
>
> It is nonobvious to me that 'next' refers to generated data.

'next' refer to python generator. It is used like that lower in the 
code. I can add a comment about it. I would be happy to do it after the 
freeze though.

> Also, I'd like this assert to have more debug information in it.

It is fairly clear once you known about bundle2 and python generator, 
the comment can probably cover that well. This is a developer oriented 
check (otherwise it would not be an assert)

Patch

diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -653,10 +653,19 @@  class bundlepart(object):
         # - False: currently generated,
         # - True: generation done.
         self._generated = None
         self.mandatory = mandatory
 
+    def copy(self):
+        """return a copy of the part
+
+        The new part have the very same content but no partid assigned yet.
+        Parts with generated data cannot be copied."""
+        assert not util.safehasattr(self.data, 'next')
+        return self.__class__(self.type, self._mandatoryparams,
+                              self._advisoryparams, self._data, self.mandatory)
+
     # methods used to defines the part content
     def __setdata(self, data):
         if self._generated is not None:
             raise error.ReadOnlyPartError('part is being generated')
         self._data = data