[Patchew-devel] [PATCH v2] Refactoring of UpdateProjectHeadView.handle()

Shubham Jain posted 1 patch 5 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/patchew-ci tags/patchew/20180523191607.34465-1-shubhamjain7495@gmail.com
api/models.py | 31 +++++++++++++++++++++++++++++++
api/views.py  | 29 +++--------------------------
2 files changed, 34 insertions(+), 26 deletions(-)
[Patchew-devel] [PATCH v2] Refactoring of UpdateProjectHeadView.handle()
Posted by Shubham Jain 5 years, 11 months ago
moved and refactored the UpdateProjectHeadView.handle() into method of api.models.Project so that it can be re-used in rest conversion of update-project-head
---
 api/models.py | 31 +++++++++++++++++++++++++++++++
 api/views.py  | 29 +++--------------------------
 2 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/api/models.py b/api/models.py
index d602cb7..1a51a6d 100644
--- a/api/models.py
+++ b/api/models.py
@@ -171,6 +171,37 @@ class Project(models.Model):
     def get_subprojects(self):
         return Project.objects.filter(parent_project=self)
 
+    def get_project_head(self):
+        return self.get_property("git.head")
+
+    def set_project_head(self, new_head):
+        self.set_property("git.head", new_head)
+
+    project_head = property(get_project_head,set_project_head)
+
+    def series_update(self, message_ids):
+        updated_series = []
+        for msgid in message_ids:
+            if msgid.startswith("<") and msgid.endswith(">"):
+                msgid = msgid[1:-1]
+            mo = Message.objects.filter(project=self, message_id=msgid,
+                                            is_merged=False).first()
+            if not mo:
+                continue
+            mo.is_merged = True
+            mo.save()
+            s = mo.get_series_head()
+            if s:
+                updated_series.append(s)
+        for s in updated_series:
+            for p in series.get_patches():
+                if not p.is_merged:
+                    break
+            else:
+                series.is_merged = True
+                series.save()
+        return len(updated_series)
+
 class ProjectProperty(models.Model):
     project = models.ForeignKey('Project', on_delete=models.CASCADE)
     name = models.CharField(max_length=1024, db_index=True)
diff --git a/api/views.py b/api/views.py
index c27cd5a..7a37481 100644
--- a/api/views.py
+++ b/api/views.py
@@ -120,34 +120,11 @@ class UpdateProjectHeadView(APILoginRequiredView):
 
     def handle(self, request, project, old_head, new_head, message_ids):
         po = Project.objects.get(name=project)
-        old_head_0 = po.get_property("git.head")
+        old_head_0 = po.project_head
         if old_head_0 and old_head_0 != old_head:
             raise Exception("wrong old head")
-        ret = 0
-        updated_series = []
-        for msgid in message_ids:
-            if msgid.startswith("<") and msgid.endswith(">"):
-                msgid = msgid[1:-1]
-            mo = Message.objects.filter(project=po, message_id=msgid,
-                                        is_merged=False).first()
-            if not mo:
-                continue
-            ret += 1
-            mo.is_merged = True
-            mo.save()
-            s = mo.get_series_head()
-            if s:
-                updated_series.append(s)
-        for s in updated_series:
-            merged = True
-            for p in s.get_patches():
-                if not p.is_merged:
-                    merged = False
-                    break
-            if merged:
-                s.is_merged = True
-                s.save()
-        po.set_property("git.head", new_head)
+        ret = po.series_update(message_ids)
+        po.project_head = new_head
         return ret
 
 class SetPropertyView(APILoginRequiredView):
-- 
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 v2] Refactoring of UpdateProjectHeadView.handle()
Posted by Paolo Bonzini 5 years, 11 months ago
On 23/05/2018 21:16, Shubham Jain wrote:
> moved and refactored the UpdateProjectHeadView.handle() into method of api.models.Project so that it can be re-used in rest conversion of update-project-head
> ---
>  api/models.py | 31 +++++++++++++++++++++++++++++++
>  api/views.py  | 29 +++--------------------------
>  2 files changed, 34 insertions(+), 26 deletions(-)
> 
> diff --git a/api/models.py b/api/models.py
> index d602cb7..1a51a6d 100644
> --- a/api/models.py
> +++ b/api/models.py
> @@ -171,6 +171,37 @@ class Project(models.Model):
>      def get_subprojects(self):
>          return Project.objects.filter(parent_project=self)
>  
> +    def get_project_head(self):
> +        return self.get_property("git.head")
> +
> +    def set_project_head(self, new_head):
> +        self.set_property("git.head", new_head)
> +
> +    project_head = property(get_project_head,set_project_head)

Fam, what do you think about making git.head a proper field in the
model, rather than a property?

Shubham, if Fam agrees, would you like to try it?  It's outside the REST
API project, but it is an interesting experience with Django to write
the migration code etc.

The patch looks okay to me - I cannot commit it right now, but perhaps
Fam can do it for me?

Thanks,

Paolo

> +
> +    def series_update(self, message_ids):
> +        updated_series = []
> +        for msgid in message_ids:
> +            if msgid.startswith("<") and msgid.endswith(">"):
> +                msgid = msgid[1:-1]
> +            mo = Message.objects.filter(project=self, message_id=msgid,
> +                                            is_merged=False).first()
> +            if not mo:
> +                continue
> +            mo.is_merged = True
> +            mo.save()
> +            s = mo.get_series_head()
> +            if s:
> +                updated_series.append(s)
> +        for s in updated_series:
> +            for p in series.get_patches():
> +                if not p.is_merged:
> +                    break
> +            else:
> +                series.is_merged = True
> +                series.save()
> +        return len(updated_series)
> +
>  class ProjectProperty(models.Model):
>      project = models.ForeignKey('Project', on_delete=models.CASCADE)
>      name = models.CharField(max_length=1024, db_index=True)
> diff --git a/api/views.py b/api/views.py
> index c27cd5a..7a37481 100644
> --- a/api/views.py
> +++ b/api/views.py
> @@ -120,34 +120,11 @@ class UpdateProjectHeadView(APILoginRequiredView):
>  
>      def handle(self, request, project, old_head, new_head, message_ids):
>          po = Project.objects.get(name=project)
> -        old_head_0 = po.get_property("git.head")
> +        old_head_0 = po.project_head
>          if old_head_0 and old_head_0 != old_head:
>              raise Exception("wrong old head")
> -        ret = 0
> -        updated_series = []
> -        for msgid in message_ids:
> -            if msgid.startswith("<") and msgid.endswith(">"):
> -                msgid = msgid[1:-1]
> -            mo = Message.objects.filter(project=po, message_id=msgid,
> -                                        is_merged=False).first()
> -            if not mo:
> -                continue
> -            ret += 1
> -            mo.is_merged = True
> -            mo.save()
> -            s = mo.get_series_head()
> -            if s:
> -                updated_series.append(s)
> -        for s in updated_series:
> -            merged = True
> -            for p in s.get_patches():
> -                if not p.is_merged:
> -                    merged = False
> -                    break
> -            if merged:
> -                s.is_merged = True
> -                s.save()
> -        po.set_property("git.head", new_head)
> +        ret = po.series_update(message_ids)
> +        po.project_head = new_head
>          return ret
>  
>  class SetPropertyView(APILoginRequiredView):
> 

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH v2] Refactoring of UpdateProjectHeadView.handle()
Posted by Fam Zheng 5 years, 11 months ago
On Thu, 05/24 18:09, Paolo Bonzini wrote:
> On 23/05/2018 21:16, Shubham Jain wrote:
> > moved and refactored the UpdateProjectHeadView.handle() into method of api.models.Project so that it can be re-used in rest conversion of update-project-head
> > ---
> >  api/models.py | 31 +++++++++++++++++++++++++++++++
> >  api/views.py  | 29 +++--------------------------
> >  2 files changed, 34 insertions(+), 26 deletions(-)
> > 
> > diff --git a/api/models.py b/api/models.py
> > index d602cb7..1a51a6d 100644
> > --- a/api/models.py
> > +++ b/api/models.py
> > @@ -171,6 +171,37 @@ class Project(models.Model):
> >      def get_subprojects(self):
> >          return Project.objects.filter(parent_project=self)
> >  
> > +    def get_project_head(self):
> > +        return self.get_property("git.head")
> > +
> > +    def set_project_head(self, new_head):
> > +        self.set_property("git.head", new_head)
> > +
> > +    project_head = property(get_project_head,set_project_head)

Please remember to add comma between parameters.

> 
> Fam, what do you think about making git.head a proper field in the
> model, rather than a property?

Yes, that makes sense to me.

> 
> Shubham, if Fam agrees, would you like to try it?  It's outside the REST
> API project, but it is an interesting experience with Django to write
> the migration code etc.
> 
> The patch looks okay to me - I cannot commit it right now, but perhaps
> Fam can do it for me?

Applied, thanks!

Fam

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH v2] Refactoring of UpdateProjectHeadView.handle()
Posted by Shubham Jain 5 years, 10 months ago
On Thu, 24 May 2018 at 9:39 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 23/05/2018 21:16, Shubham Jain wrote:
> > moved and refactored the UpdateProjectHeadView.handle() into method of
> api.models.Project so that it can be re-used in rest conversion of
> update-project-head
> > ---
> >  api/models.py | 31 +++++++++++++++++++++++++++++++
> >  api/views.py  | 29 +++--------------------------
> >  2 files changed, 34 insertions(+), 26 deletions(-)
> >
> > diff --git a/api/models.py b/api/models.py
> > index d602cb7..1a51a6d 100644
> > --- a/api/models.py
> > +++ b/api/models.py
> > @@ -171,6 +171,37 @@ class Project(models.Model):
> >      def get_subprojects(self):
> >          return Project.objects.filter(parent_project=self)
> >
> > +    def get_project_head(self):
> > +        return self.get_property("git.head")
> > +
> > +    def set_project_head(self, new_head):
> > +        self.set_property("git.head", new_head)
> > +
> > +    project_head = property(get_project_head,set_project_head)
>
> Fam, what do you think about making git.head a proper field in the
> model, rather than a property?
>
> Shubham, if Fam agrees, would you like to try it?  It's outside the REST
> API project, but it is an interesting experience with Django to write
> the migration code etc.

Yes. Can you elaborate?

>
> The patch looks okay to me - I cannot commit it right now, but perhaps
> Fam can do it for me?
>
> Thanks,
>
> Paolo
>
> > +
> > +    def series_update(self, message_ids):
> > +        updated_series = []
> > +        for msgid in message_ids:
> > +            if msgid.startswith("<") and msgid.endswith(">"):
> > +                msgid = msgid[1:-1]
> > +            mo = Message.objects.filter(project=self, message_id=msgid,
> > +                                            is_merged=False).first()
> > +            if not mo:
> > +                continue
> > +            mo.is_merged = True
> > +            mo.save()
> > +            s = mo.get_series_head()
> > +            if s:
> > +                updated_series.append(s)
> > +        for s in updated_series:
> > +            for p in series.get_patches():
> > +                if not p.is_merged:
> > +                    break
> > +            else:
> > +                series.is_merged = True
> > +                series.save()
> > +        return len(updated_series)
> > +
> >  class ProjectProperty(models.Model):
> >      project = models.ForeignKey('Project', on_delete=models.CASCADE)
> >      name = models.CharField(max_length=1024, db_index=True)
> > diff --git a/api/views.py b/api/views.py
> > index c27cd5a..7a37481 100644
> > --- a/api/views.py
> > +++ b/api/views.py
> > @@ -120,34 +120,11 @@ class UpdateProjectHeadView(APILoginRequiredView):
> >
> >      def handle(self, request, project, old_head, new_head, message_ids):
> >          po = Project.objects.get(name=project)
> > -        old_head_0 = po.get_property("git.head")
> > +        old_head_0 = po.project_head
> >          if old_head_0 and old_head_0 != old_head:
> >              raise Exception("wrong old head")
> > -        ret = 0
> > -        updated_series = []
> > -        for msgid in message_ids:
> > -            if msgid.startswith("<") and msgid.endswith(">"):
> > -                msgid = msgid[1:-1]
> > -            mo = Message.objects.filter(project=po, message_id=msgid,
> > -                                        is_merged=False).first()
> > -            if not mo:
> > -                continue
> > -            ret += 1
> > -            mo.is_merged = True
> > -            mo.save()
> > -            s = mo.get_series_head()
> > -            if s:
> > -                updated_series.append(s)
> > -        for s in updated_series:
> > -            merged = True
> > -            for p in s.get_patches():
> > -                if not p.is_merged:
> > -                    merged = False
> > -                    break
> > -            if merged:
> > -                s.is_merged = True
> > -                s.save()
> > -        po.set_property("git.head", new_head)
> > +        ret = po.series_update(message_ids)
> > +        po.project_head = new_head
> >          return ret
> >
> >  class SetPropertyView(APILoginRequiredView):
> >

Also, if one of you can elaborate the use of pluginmethod field. I tried
going through code and experiment around it but it wasn’t a much help.
Thanks
Shubham
_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH v2] Refactoring of UpdateProjectHeadView.handle()
Posted by Paolo Bonzini 5 years, 10 months ago
On 25/05/2018 17:57, Shubham Jain wrote:
> 
> On Thu, 24 May 2018 at 9:39 PM, Paolo Bonzini <pbonzini@redhat.com
> <mailto:pbonzini@redhat.com>> wrote:
> 
>     On 23/05/2018 21:16, Shubham Jain wrote:
>     > moved and refactored the UpdateProjectHeadView.handle() into
>     method of api.models.Project so that it can be re-used in rest
>     conversion of update-project-head
>     > ---
>     >  api/models.py | 31 +++++++++++++++++++++++++++++++
>     >  api/views.py  | 29 +++--------------------------
>     >  2 files changed, 34 insertions(+), 26 deletions(-)
>     >
>     > diff --git a/api/models.py b/api/models.py
>     > index d602cb7..1a51a6d 100644
>     > --- a/api/models.py
>     > +++ b/api/models.py
>     > @@ -171,6 +171,37 @@ class Project(models.Model):
>     >      def get_subprojects(self):
>     >          return Project.objects.filter(parent_project=self)
>     > 
>     > +    def get_project_head(self):
>     > +        return self.get_property("git.head")
>     > +
>     > +    def set_project_head(self, new_head):
>     > +        self.set_property("git.head", new_head)
>     > +
>     > +    project_head = property(get_project_head,set_project_head)
> 
>     Fam, what do you think about making git.head a proper field in the
>     model, rather than a property?
> 
>     Shubham, if Fam agrees, would you like to try it?  It's outside the REST
>     API project, but it is an interesting experience with Django to write
>     the migration code etc.
> 
> Yes. Can you elaborate? 

Let's finish this endpoint first. :)  In the meanwhile I'll have more
infrastructure committed through the big Result refactoring that I
posted a few days ago.

> Also, if one of you can elaborate the use of pluginmethod field. I tried
> going through code and experiment around it but it wasn’t a much help. 

PluginMethodField is like SerializerMethodField, but it calls a method
on another object (the plugin).  There are some examples, such as in
mods/tags.py if I remember correctly.  What are your doubts?

Paolo

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH v2] Refactoring of UpdateProjectHeadView.handle()
Posted by Shubham Jain 5 years, 10 months ago
On Fri, 25 May 2018 at 9:42 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 25/05/2018 17:57, Shubham Jain wrote:
> >
> > On Thu, 24 May 2018 at 9:39 PM, Paolo Bonzini <pbonzini@redhat.com
> > <mailto:pbonzini@redhat.com>> wrote:
> >
> >     On 23/05/2018 21:16, Shubham Jain wrote:
> >     > moved and refactored the UpdateProjectHeadView.handle() into
> >     method of api.models.Project so that it can be re-used in rest
> >     conversion of update-project-head
> >     > ---
> >     >  api/models.py | 31 +++++++++++++++++++++++++++++++
> >     >  api/views.py  | 29 +++--------------------------
> >     >  2 files changed, 34 insertions(+), 26 deletions(-)
> >     >
> >     > diff --git a/api/models.py b/api/models.py
> >     > index d602cb7..1a51a6d 100644
> >     > --- a/api/models.py
> >     > +++ b/api/models.py
> >     > @@ -171,6 +171,37 @@ class Project(models.Model):
> >     >      def get_subprojects(self):
> >     >          return Project.objects.filter(parent_project=self)
> >     >
> >     > +    def get_project_head(self):
> >     > +        return self.get_property("git.head")
> >     > +
> >     > +    def set_project_head(self, new_head):
> >     > +        self.set_property("git.head", new_head)
> >     > +
> >     > +    project_head = property(get_project_head,set_project_head)
> >
> >     Fam, what do you think about making git.head a proper field in the
> >     model, rather than a property?
> >
> >     Shubham, if Fam agrees, would you like to try it?  It's outside the
> REST
> >     API project, but it is an interesting experience with Django to write
> >     the migration code etc.
> >
> > Yes. Can you elaborate?
>
> Let's finish this endpoint first. :)  In the meanwhile I'll have more
> infrastructure committed through the big Result refactoring that I
> posted a few days ago.
>
> > Also, if one of you can elaborate the use of pluginmethod field. I tried
> > going through code and experiment around it but it wasn’t a much help.
>
> PluginMethodField is like SerializerMethodField, but it calls a method
> on another object (the plugin).  There are some examples, such as in
> mods/tags.py if I remember correctly.  What are your doubts?

I am not  able to get the intuition behind it, like you would get with
another things for eg a serialiser method field. How is it used? And how it
helps in getting the desired results?

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