[Patchew-devel] [PATCH 1/3] Usage of getters and setters to get mbox field

[Paolo Bonzini]

On 08/05/2018 16:21, Shubham Jain wrote:
> Sure. 
> 
> To have a Message object with a field mbox which can be simply used like
> msg.mbox setter and getter of mbox are added. And to free the variable
> mbox all the existing ones are changed to mbox_blob. 
> This would also help the rest api to directly fetch the mbox content
> with a single field.

This is missing that we want to avoid the SerializerMethodField.
Second, usually the imperative "do X" is preferred to the passive ("X is
done"):

In order to use a CharField instead of a SerializerMethodField (which is
read-only and gets in the way when adding POST support), turn the
Message object's get_"mbox()" method into a property that can be simply
used like "msg.mbox".  In order to free the "mbox" name, all the
existing occurrences are changed to mbox_blob.

Applied the first two patches, thanks!

Paolo

> 
> On Tue, May 8, 2018 at 6:51 PM Paolo Bonzini <pbonzini at redhat.com
> <mailto:pbonzini at redhat.com>> wrote:
> 
>     On 08/05/2018 13:50, Shubham Jain wrote:
>     > Rename the existing "mbox" field to e.g. "mbox_blob" and add
>     getters and setters for "mbox"
> 
>     The same remark about the "why" holds here.  Would you like to try
>     (re)writing this commit message yourself?  Just replying to this mail
>     will do.
> 
>     Thanks!
> 
>     Paolo
> 
>     > ---
>     >  api/models.py | 22 ++++++++++++++--------
>     >  1 file changed, 14 insertions(+), 8 deletions(-)
>     >
>     > diff --git a/api/models.py b/api/models.py
>     > index 504f2c7..e161325 100644
>     > --- a/api/models.py
>     > +++ b/api/models.py
>     > @@ -321,20 +321,26 @@ class Message(models.Model):
>     > 
>     >      objects = MessageManager()
>     > 
>     > -    def save_mbox(self, mbox):
>     > -        save_blob(mbox, self.message_id)
>     > +    def save_mbox(self, mbox_blob):
>     > +        save_blob(mbox_blob, self.message_id)
>     > 
>     >      def get_mbox_obj(self):
>     >          self.get_mbox()
>     >          return self._mbox_obj
>     > 
>     >      def get_mbox(self):
>     > -        if hasattr(self, "mbox"):
>     > -            return self.mbox
>     > -        self.mbox = load_blob(self.message_id)
>     > -        self._mbox_obj = MboxMessage(self.mbox)
>     > -        return self.mbox
>     > -
>     > +        if hasattr(self, "mbox_blob"):
>     > +            return self.mbox_blob
>     > +        self.mbox_blob = load_blob(self.message_id)
>     > +        self._mbox_obj = MboxMessage(self.mbox_blob)
>     > +        return self.mbox_blob
>     > +   
>     > +    mbox = property(get_mbox)
>     > +
>     > +    @mbox.setter
>     > +    def mbox(self, value):
>     > +        self.mbox_blob = value
>     > +   
>     >      def get_num(self):
>     >          assert self.is_patch or self.is_series_head
>     >          cur, total = 1, 1
>     >
>