[Patchew-devel] [PATCH] Change "mbox" to use getters and setters instead of SerializerMethodField - Rename the existing "mbox" field to e.g. "mbox_blob" - Add getters and setters for "mbox" to api.models.Message

Shubham Jain posted 1 patch 6 years, 2 months ago
Failed in applying to current master (apply log)
api/models.py | 24 ++++++++++++++----------
api/rest.py   |  5 +----
2 files changed, 15 insertions(+), 14 deletions(-)
[Patchew-devel] [PATCH] Change "mbox" to use getters and setters instead of SerializerMethodField - Rename the existing "mbox" field to e.g. "mbox_blob" - Add getters and setters for "mbox" to api.models.Message
Posted by Shubham Jain 6 years, 2 months ago
---
 api/models.py | 24 ++++++++++++++----------
 api/rest.py   |  5 +----
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/api/models.py b/api/models.py
index 4cc2b74..13ec44e 100644
--- a/api/models.py
+++ b/api/models.py
@@ -245,12 +245,12 @@ class MessageManager(models.Manager):
             self.delete_subthread(r)
         msg.delete()
 
-    def add_message_from_mbox(self, mbox, user, project_name=None):
+    def add_message_from_mbox(self, mbox_blob, user, project_name=None):
 
         def find_message_projects(m):
             return [p for p in Project.objects.all() if p.recognizes(m)]
 
-        m = MboxMessage(mbox)
+        m = MboxMessage(mbox_blob)
         msgid = m.get_message_id()
         if project_name:
             projects = [Project.object.get(name=project_name)]
@@ -272,7 +272,7 @@ class MessageManager(models.Manager):
             msg.project = p
             if self.filter(message_id=msgid, project__name=p.name).first():
                 raise self.DuplicateMessageError(msgid)
-            msg.save_mbox(mbox)
+            msg.save_mbox(mbox_blob)
             msg.save()
             emit_event("MessageAdded", message=msg)
             self.update_series(msg)
@@ -318,20 +318,24 @@ class Message(models.Model):
     num_patches = models.IntegerField(null=False, default=-1, blank=True)
 
     objects = MessageManager()
+    def _get_mbox_blob(self):
+        return self.get_mbox()    
 
-    def save_mbox(self, mbox):
-        save_blob(mbox, self.message_id)
+    mbox = property(_get_mbox_blob)
+
+    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
 
     def get_num(self):
         assert self.is_patch or self.is_series_head
diff --git a/api/rest.py b/api/rest.py
index 5610844..a2ab004 100644
--- a/api/rest.py
+++ b/api/rest.py
@@ -287,10 +287,7 @@ class MessageSerializer(BaseMessageSerializer):
     class Meta:
         model = Message
         fields = BaseMessageSerializer.Meta.fields + ('mbox', )
-    def get_mbox(self, obj):
-        return obj.get_mbox()
-    mbox = SerializerMethodField()
-
+        
     def get_fields(self):
         fields = super(MessageSerializer, self).get_fields()
         request = self.context['request']
-- 
2.14.3 (Apple Git-98)

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH] Change "mbox" to use getters and setters instead of SerializerMethodField - Rename the existing "mbox" field to e.g. "mbox_blob" - Add getters and setters for "mbox" to api.models.Message
Posted by Paolo Bonzini 6 years, 2 months ago
On 01/05/2018 09:35, Shubham Jain wrote:
> ---
>  api/models.py | 24 ++++++++++++++----------
>  api/rest.py   |  5 +----
>  2 files changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/api/models.py b/api/models.py
> index 4cc2b74..13ec44e 100644
> --- a/api/models.py
> +++ b/api/models.py
> @@ -245,12 +245,12 @@ class MessageManager(models.Manager):
>              self.delete_subthread(r)
>          msg.delete()
>  
> -    def add_message_from_mbox(self, mbox, user, project_name=None):
> +    def add_message_from_mbox(self, mbox_blob, user, project_name=None):
>  
>          def find_message_projects(m):
>              return [p for p in Project.objects.all() if p.recognizes(m)]
>  
> -        m = MboxMessage(mbox)
> +        m = MboxMessage(mbox_blob)
>          msgid = m.get_message_id()
>          if project_name:
>              projects = [Project.object.get(name=project_name)]
> @@ -272,7 +272,7 @@ class MessageManager(models.Manager):
>              msg.project = p
>              if self.filter(message_id=msgid, project__name=p.name).first():
>                  raise self.DuplicateMessageError(msgid)
> -            msg.save_mbox(mbox)
> +            msg.save_mbox(mbox_blob)
>              msg.save()
>              emit_event("MessageAdded", message=msg)
>              self.update_series(msg)
> @@ -318,20 +318,24 @@ class Message(models.Model):
>      num_patches = models.IntegerField(null=False, default=-1, blank=True)
>  
>      objects = MessageManager()
> +    def _get_mbox_blob(self):
> +        return self.get_mbox()    
>  
> -    def save_mbox(self, mbox):
> -        save_blob(mbox, self.message_id)
> +    mbox = property(_get_mbox_blob)

This can be simply "mbox = property(get_mbox)", right?  There should be
no need to add _get_mbox_blob.  After this is changed, we can look at
adding a setter for the mbox property.

Fam, could you please review this patch?  It's more your area. :)

Paolo

> +    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
>  
>      def get_num(self):
>          assert self.is_patch or self.is_series_head
> diff --git a/api/rest.py b/api/rest.py
> index 5610844..a2ab004 100644
> --- a/api/rest.py
> +++ b/api/rest.py
> @@ -287,10 +287,7 @@ class MessageSerializer(BaseMessageSerializer):
>      class Meta:
>          model = Message
>          fields = BaseMessageSerializer.Meta.fields + ('mbox', )
> -    def get_mbox(self, obj):
> -        return obj.get_mbox()
> -    mbox = SerializerMethodField()
> -
> +        
>      def get_fields(self):
>          fields = super(MessageSerializer, self).get_fields()
>          request = self.context['request']
> 

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH] Change "mbox" to use getters and setters instead of SerializerMethodField - Rename the existing "mbox" field to e.g. "mbox_blob" - Add getters and setters for "mbox" to api.models.Message
Posted by Fam Zheng 6 years, 2 months ago
On Wed, 05/02 12:01, Paolo Bonzini wrote:
> On 01/05/2018 09:35, Shubham Jain wrote:
> > ---
> >  api/models.py | 24 ++++++++++++++----------
> >  api/rest.py   |  5 +----
> >  2 files changed, 15 insertions(+), 14 deletions(-)
> > 
> > diff --git a/api/models.py b/api/models.py
> > index 4cc2b74..13ec44e 100644
> > --- a/api/models.py
> > +++ b/api/models.py
> > @@ -245,12 +245,12 @@ class MessageManager(models.Manager):
> >              self.delete_subthread(r)
> >          msg.delete()
> >  
> > -    def add_message_from_mbox(self, mbox, user, project_name=None):
> > +    def add_message_from_mbox(self, mbox_blob, user, project_name=None):
> >  
> >          def find_message_projects(m):
> >              return [p for p in Project.objects.all() if p.recognizes(m)]
> >  
> > -        m = MboxMessage(mbox)
> > +        m = MboxMessage(mbox_blob)
> >          msgid = m.get_message_id()
> >          if project_name:
> >              projects = [Project.object.get(name=project_name)]
> > @@ -272,7 +272,7 @@ class MessageManager(models.Manager):
> >              msg.project = p
> >              if self.filter(message_id=msgid, project__name=p.name).first():
> >                  raise self.DuplicateMessageError(msgid)
> > -            msg.save_mbox(mbox)
> > +            msg.save_mbox(mbox_blob)
> >              msg.save()
> >              emit_event("MessageAdded", message=msg)
> >              self.update_series(msg)
> > @@ -318,20 +318,24 @@ class Message(models.Model):
> >      num_patches = models.IntegerField(null=False, default=-1, blank=True)
> >  
> >      objects = MessageManager()
> > +    def _get_mbox_blob(self):
> > +        return self.get_mbox()    
> >  
> > -    def save_mbox(self, mbox):
> > -        save_blob(mbox, self.message_id)
> > +    mbox = property(_get_mbox_blob)
> 
> This can be simply "mbox = property(get_mbox)", right?  There should be
> no need to add _get_mbox_blob.  After this is changed, we can look at
> adding a setter for the mbox property.
> 
> Fam, could you please review this patch?  It's more your area. :)

Looks good to me, except the long subject could be broken into a proper commit
message. :)

Fam

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH] Change "mbox" to use getters and setters instead of SerializerMethodField - Rename the existing "mbox" field to e.g. "mbox_blob" - Add getters and setters for "mbox" to api.models.Message
Posted by Shubham Jain 6 years, 2 months ago
Pushed it by mistake. Is there any way to revert it?

On Wed 2 May, 2018 7:24 pm Fam Zheng, <famz@redhat.com> wrote:

> On Wed, 05/02 12:01, Paolo Bonzini wrote:
> > On 01/05/2018 09:35, Shubham Jain wrote:
> > > ---
> > >  api/models.py | 24 ++++++++++++++----------
> > >  api/rest.py   |  5 +----
> > >  2 files changed, 15 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/api/models.py b/api/models.py
> > > index 4cc2b74..13ec44e 100644
> > > --- a/api/models.py
> > > +++ b/api/models.py
> > > @@ -245,12 +245,12 @@ class MessageManager(models.Manager):
> > >              self.delete_subthread(r)
> > >          msg.delete()
> > >
> > > -    def add_message_from_mbox(self, mbox, user, project_name=None):
> > > +    def add_message_from_mbox(self, mbox_blob, user,
> project_name=None):
> > >
> > >          def find_message_projects(m):
> > >              return [p for p in Project.objects.all() if
> p.recognizes(m)]
> > >
> > > -        m = MboxMessage(mbox)
> > > +        m = MboxMessage(mbox_blob)
> > >          msgid = m.get_message_id()
> > >          if project_name:
> > >              projects = [Project.object.get(name=project_name)]
> > > @@ -272,7 +272,7 @@ class MessageManager(models.Manager):
> > >              msg.project = p
> > >              if self.filter(message_id=msgid, project__name=p.name
> ).first():
> > >                  raise self.DuplicateMessageError(msgid)
> > > -            msg.save_mbox(mbox)
> > > +            msg.save_mbox(mbox_blob)
> > >              msg.save()
> > >              emit_event("MessageAdded", message=msg)
> > >              self.update_series(msg)
> > > @@ -318,20 +318,24 @@ class Message(models.Model):
> > >      num_patches = models.IntegerField(null=False, default=-1,
> blank=True)
> > >
> > >      objects = MessageManager()
> > > +    def _get_mbox_blob(self):
> > > +        return self.get_mbox()
> > >
> > > -    def save_mbox(self, mbox):
> > > -        save_blob(mbox, self.message_id)
> > > +    mbox = property(_get_mbox_blob)
> >
> > This can be simply "mbox = property(get_mbox)", right?  There should be
> > no need to add _get_mbox_blob.  After this is changed, we can look at
> > adding a setter for the mbox property.
> >
> > Fam, could you please review this patch?  It's more your area. :)
>
> Looks good to me, except the long subject could be broken into a proper
> commit
> message. :)
>
> Fam
>
_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH] Change "mbox" to use getters and setters instead of SerializerMethodField - Rename the existing "mbox" field to e.g. "mbox_blob" - Add getters and setters for "mbox" to api.models.Message
Posted by Paolo Bonzini 6 years, 2 months ago
On 02/05/2018 15:55, Shubham Jain wrote:
> Pushed it by mistake. Is there any way to revert it? 

You didn't push to patchew.org, don't worry. :)

I don't see it at https://github.com/shubhamdotjain/patchew either, what
is the issue exactly?

Thanks,

Paolo

> On Wed 2 May, 2018 7:24 pm Fam Zheng, <famz@redhat.com
> <mailto:famz@redhat.com>> wrote:
> 
>     On Wed, 05/02 12:01, Paolo Bonzini wrote:
>     > On 01/05/2018 09:35, Shubham Jain wrote:
>     > > ---
>     > >  api/models.py | 24 ++++++++++++++----------
>     > >  api/rest.py   |  5 +----
>     > >  2 files changed, 15 insertions(+), 14 deletions(-)
>     > >
>     > > diff --git a/api/models.py b/api/models.py
>     > > index 4cc2b74..13ec44e 100644
>     > > --- a/api/models.py
>     > > +++ b/api/models.py
>     > > @@ -245,12 +245,12 @@ class MessageManager(models.Manager):
>     > >              self.delete_subthread(r)
>     > >          msg.delete()
>     > > 
>     > > -    def add_message_from_mbox(self, mbox, user, project_name=None):
>     > > +    def add_message_from_mbox(self, mbox_blob, user,
>     project_name=None):
>     > > 
>     > >          def find_message_projects(m):
>     > >              return [p for p in Project.objects.all() if
>     p.recognizes(m)]
>     > > 
>     > > -        m = MboxMessage(mbox)
>     > > +        m = MboxMessage(mbox_blob)
>     > >          msgid = m.get_message_id()
>     > >          if project_name:
>     > >              projects = [Project.object.get(name=project_name)]
>     > > @@ -272,7 +272,7 @@ class MessageManager(models.Manager):
>     > >              msg.project = p
>     > >              if self.filter(message_id=msgid,
>     project__name=p.name <http://p.name>).first():
>     > >                  raise self.DuplicateMessageError(msgid)
>     > > -            msg.save_mbox(mbox)
>     > > +            msg.save_mbox(mbox_blob)
>     > >              msg.save()
>     > >              emit_event("MessageAdded", message=msg)
>     > >              self.update_series(msg)
>     > > @@ -318,20 +318,24 @@ class Message(models.Model):
>     > >      num_patches = models.IntegerField(null=False, default=-1,
>     blank=True)
>     > > 
>     > >      objects = MessageManager()
>     > > +    def _get_mbox_blob(self):
>     > > +        return self.get_mbox()   
>     > > 
>     > > -    def save_mbox(self, mbox):
>     > > -        save_blob(mbox, self.message_id)
>     > > +    mbox = property(_get_mbox_blob)
>     >
>     > This can be simply "mbox = property(get_mbox)", right?  There
>     should be
>     > no need to add _get_mbox_blob.  After this is changed, we can look at
>     > adding a setter for the mbox property.
>     >
>     > Fam, could you please review this patch?  It's more your area. :)
> 
>     Looks good to me, except the long subject could be broken into a
>     proper commit
>     message. :)
> 
>     Fam
> 

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel