[Patchew-devel] [PATCH v4 3/4] Make Delete Command use REST

Shubham Jain posted 4 patches 6 years, 2 months ago
There is a newer version of this series
[Patchew-devel] [PATCH v4 3/4] Make Delete Command use REST
Posted by Shubham Jain 6 years, 2 months ago
Make patchew-cli's delete use REST api and added the missing test for delete
---
 api/rest.py           |  2 ++
 patchew-cli           | 15 ++++++++++++++-
 tests/patchewtest.py  |  3 +++
 tests/test_message.py | 13 +++++++++++++
 4 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/api/rest.py b/api/rest.py
index 39bdda0..c7818c4 100644
--- a/api/rest.py
+++ b/api/rest.py
@@ -320,6 +320,8 @@ class SeriesViewSet(BaseMessageViewSet):
 
 class ProjectSeriesViewSet(ProjectMessagesViewSetMixin,
                            SeriesViewSet, mixins.DestroyModelMixin):
+    authentication_classes = (CsrfExemptSessionAuthentication, )
+    
     def collect_patches(self, series):
         if series.is_patch:
             patches = [series]
diff --git a/patchew-cli b/patchew-cli
index 9d9fde0..509bb62 100755
--- a/patchew-cli
+++ b/patchew-cli
@@ -181,7 +181,20 @@ class DeleteCommand(SubCommand):
         if not argv and not args.all:
             print("Must specify --all to delete all patches")
             return 1
-        self.api_do("delete", terms=argv)
+        series_list = []
+        if args.all:
+            resp = self.rest_api_do("projects")
+            for project in len(resp['result']):
+                url = project['series']
+                series_resp = self.rest_api_do(url)
+                series_list = series_list + series_resp['results']
+        else:
+            for term in argv:
+                series_resp = self.rest_api_do("series/?q%s"%term)
+            series_list =  series_list + series_resp['results']
+        for series in series_list:
+            url = series['resource_uri']
+            self.rest_api_do(url, request_method='delete')
         return 0
 
 class ImportCommand(SubCommand):
diff --git a/tests/patchewtest.py b/tests/patchewtest.py
index 5bed3b9..847b3f9 100644
--- a/tests/patchewtest.py
+++ b/tests/patchewtest.py
@@ -100,6 +100,9 @@ class PatchewTestCase(django.test.LiveServerTestCase):
     def cli_import(self, mbox, rc=0):
         self.check_cli(["import", self.get_data_path(mbox)], rc)
 
+    def cli_delete(self, terms, rc=0):
+        self.check_cli(["delete", terms], rc)
+
     def get_data_path(self, fname):
         r = tempfile.NamedTemporaryFile(dir=RUN_DIR, prefix="test-data-", delete=False)
         d = os.path.join(BASE_DIR, "tests", "data", fname)
diff --git a/tests/test_message.py b/tests/test_message.py
index 9448aee..ae308a5 100755
--- a/tests/test_message.py
+++ b/tests/test_message.py
@@ -10,6 +10,7 @@
 
 import time
 import datetime
+import json
 from tests.patchewtest import PatchewTestCase, main
 
 class ProjectTest(PatchewTestCase):
@@ -54,5 +55,17 @@ class ProjectTest(PatchewTestCase):
         asctime = message.get_asctime()
         self.assertEqual(asctime, "Sat Oct 22 9:06:04 2016")
 
+    def test_delete(self):
+        self.cli_login()
+        self.add_project("QEMU", "qemu-devel@nongnu.org")
+        self.cli_import("0002-unusual-cased-tags.mbox.gz")
+        a, b = self.check_cli(["search", "-r", "-o", "subject,properties"])
+        ao = json.loads(a)[0]
+        self.assertEqual(["Fam Zheng", "famz@redhat.com"],
+                         ao["properties"]["reviewers"][0])
+        self.cli_delete("from:Fam")
+        a, b = self.check_cli(["search", "-r", "-o", "subject,properties"])
+        self.assertEqual("", a)
+
 if __name__ == '__main__':
     main()
-- 
2.15.1 (Apple Git-101)

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH v4 3/4] Make Delete Command use REST
Posted by Fam Zheng 6 years, 1 month ago
On Sat, 07/07 00:02, Shubham Jain wrote:
> Make patchew-cli's delete use REST api and added the missing test for delete
> ---
>  api/rest.py           |  2 ++
>  patchew-cli           | 15 ++++++++++++++-
>  tests/patchewtest.py  |  3 +++
>  tests/test_message.py | 13 +++++++++++++
>  4 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/api/rest.py b/api/rest.py
> index 39bdda0..c7818c4 100644
> --- a/api/rest.py
> +++ b/api/rest.py
> @@ -320,6 +320,8 @@ class SeriesViewSet(BaseMessageViewSet):
>  
>  class ProjectSeriesViewSet(ProjectMessagesViewSetMixin,
>                             SeriesViewSet, mixins.DestroyModelMixin):
> +    authentication_classes = (CsrfExemptSessionAuthentication, )
> +    
>      def collect_patches(self, series):
>          if series.is_patch:
>              patches = [series]
> diff --git a/patchew-cli b/patchew-cli
> index 9d9fde0..509bb62 100755
> --- a/patchew-cli
> +++ b/patchew-cli
> @@ -181,7 +181,20 @@ class DeleteCommand(SubCommand):
>          if not argv and not args.all:
>              print("Must specify --all to delete all patches")
>              return 1
> -        self.api_do("delete", terms=argv)
> +        series_list = []
> +        if args.all:
> +            resp = self.rest_api_do("projects")
> +            for project in len(resp['result']):
> +                url = project['series']
> +                series_resp = self.rest_api_do(url)
> +                series_list = series_list + series_resp['results']
> +        else:
> +            for term in argv:
> +                series_resp = self.rest_api_do("series/?q%s"%term)

Whitespaces around %, please.

> +            series_list =  series_list + series_resp['results']
> +        for series in series_list:
> +            url = series['resource_uri']
> +            self.rest_api_do(url, request_method='delete')

Just a general note with this one as an example: the transferred data between
server and cli is significantly increased because we are switching to the
RESTful primitives. This is something we should think about.

For example, it used to be very easy to delete 30% or even 100% out of 10k
messages with some widely matched criteria, such as '>1m'. But after the
conversion, I'm not sure if that still works.  I personally don't do much
administritive work from cli, but such a scenario was a consideration of the old
API.

>          return 0
>  
>  class ImportCommand(SubCommand):
> diff --git a/tests/patchewtest.py b/tests/patchewtest.py
> index 5bed3b9..847b3f9 100644
> --- a/tests/patchewtest.py
> +++ b/tests/patchewtest.py
> @@ -100,6 +100,9 @@ class PatchewTestCase(django.test.LiveServerTestCase):
>      def cli_import(self, mbox, rc=0):
>          self.check_cli(["import", self.get_data_path(mbox)], rc)
>  
> +    def cli_delete(self, terms, rc=0):
> +        self.check_cli(["delete", terms], rc)
> +
>      def get_data_path(self, fname):
>          r = tempfile.NamedTemporaryFile(dir=RUN_DIR, prefix="test-data-", delete=False)
>          d = os.path.join(BASE_DIR, "tests", "data", fname)
> diff --git a/tests/test_message.py b/tests/test_message.py
> index 9448aee..ae308a5 100755
> --- a/tests/test_message.py
> +++ b/tests/test_message.py
> @@ -10,6 +10,7 @@
>  
>  import time
>  import datetime
> +import json
>  from tests.patchewtest import PatchewTestCase, main
>  
>  class ProjectTest(PatchewTestCase):
> @@ -54,5 +55,17 @@ class ProjectTest(PatchewTestCase):
>          asctime = message.get_asctime()
>          self.assertEqual(asctime, "Sat Oct 22 9:06:04 2016")
>  
> +    def test_delete(self):
> +        self.cli_login()
> +        self.add_project("QEMU", "qemu-devel@nongnu.org")
> +        self.cli_import("0002-unusual-cased-tags.mbox.gz")
> +        a, b = self.check_cli(["search", "-r", "-o", "subject,properties"])
> +        ao = json.loads(a)[0]
> +        self.assertEqual(["Fam Zheng", "famz@redhat.com"],
> +                         ao["properties"]["reviewers"][0])
> +        self.cli_delete("from:Fam")
> +        a, b = self.check_cli(["search", "-r", "-o", "subject,properties"])
> +        self.assertEqual("", a)

While you are at it, please import another message that will not be deleted by
the term, and verify it survives the detele command.

Fam

> +
>  if __name__ == '__main__':
>      main()
> -- 
> 2.15.1 (Apple Git-101)
> 
> _______________________________________________
> Patchew-devel mailing list
> Patchew-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/patchew-devel

_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH v4 3/4] Make Delete Command use REST
Posted by Shubham Jain 6 years, 1 month ago
Can we test this somehow?

On Tue, Jul 10, 2018 at 1:20 PM Fam Zheng <famz@redhat.com> wrote:

> On Sat, 07/07 00:02, Shubham Jain wrote:
> > Make patchew-cli's delete use REST api and added the missing test for
> delete
> > ---
> >  api/rest.py           |  2 ++
> >  patchew-cli           | 15 ++++++++++++++-
> >  tests/patchewtest.py  |  3 +++
> >  tests/test_message.py | 13 +++++++++++++
> >  4 files changed, 32 insertions(+), 1 deletion(-)
> >
> > diff --git a/api/rest.py b/api/rest.py
> > index 39bdda0..c7818c4 100644
> > --- a/api/rest.py
> > +++ b/api/rest.py
> > @@ -320,6 +320,8 @@ class SeriesViewSet(BaseMessageViewSet):
> >
> >  class ProjectSeriesViewSet(ProjectMessagesViewSetMixin,
> >                             SeriesViewSet, mixins.DestroyModelMixin):
> > +    authentication_classes = (CsrfExemptSessionAuthentication, )
> > +
> >      def collect_patches(self, series):
> >          if series.is_patch:
> >              patches = [series]
> > diff --git a/patchew-cli b/patchew-cli
> > index 9d9fde0..509bb62 100755
> > --- a/patchew-cli
> > +++ b/patchew-cli
> > @@ -181,7 +181,20 @@ class DeleteCommand(SubCommand):
> >          if not argv and not args.all:
> >              print("Must specify --all to delete all patches")
> >              return 1
> > -        self.api_do("delete", terms=argv)
> > +        series_list = []
> > +        if args.all:
> > +            resp = self.rest_api_do("projects")
> > +            for project in len(resp['result']):
> > +                url = project['series']
> > +                series_resp = self.rest_api_do(url)
> > +                series_list = series_list + series_resp['results']
> > +        else:
> > +            for term in argv:
> > +                series_resp = self.rest_api_do("series/?q%s"%term)
>
> Whitespaces around %, please.
>
> > +            series_list =  series_list + series_resp['results']
> > +        for series in series_list:
> > +            url = series['resource_uri']
> > +            self.rest_api_do(url, request_method='delete')
>
> Just a general note with this one as an example: the transferred data
> between
> server and cli is significantly increased because we are switching to the
> RESTful primitives. This is something we should think about.
>
> For example, it used to be very easy to delete 30% or even 100% out of 10k
> messages with some widely matched criteria, such as '>1m'. But after the
> conversion, I'm not sure if that still works.  I personally don't do much
> administritive work from cli, but such a scenario was a consideration of
> the old
> API.
>
> >          return 0
> >
> >  class ImportCommand(SubCommand):
> > diff --git a/tests/patchewtest.py b/tests/patchewtest.py
> > index 5bed3b9..847b3f9 100644
> > --- a/tests/patchewtest.py
> > +++ b/tests/patchewtest.py
> > @@ -100,6 +100,9 @@ class
> PatchewTestCase(django.test.LiveServerTestCase):
> >      def cli_import(self, mbox, rc=0):
> >          self.check_cli(["import", self.get_data_path(mbox)], rc)
> >
> > +    def cli_delete(self, terms, rc=0):
> > +        self.check_cli(["delete", terms], rc)
> > +
> >      def get_data_path(self, fname):
> >          r = tempfile.NamedTemporaryFile(dir=RUN_DIR,
> prefix="test-data-", delete=False)
> >          d = os.path.join(BASE_DIR, "tests", "data", fname)
> > diff --git a/tests/test_message.py b/tests/test_message.py
> > index 9448aee..ae308a5 100755
> > --- a/tests/test_message.py
> > +++ b/tests/test_message.py
> > @@ -10,6 +10,7 @@
> >
> >  import time
> >  import datetime
> > +import json
> >  from tests.patchewtest import PatchewTestCase, main
> >
> >  class ProjectTest(PatchewTestCase):
> > @@ -54,5 +55,17 @@ class ProjectTest(PatchewTestCase):
> >          asctime = message.get_asctime()
> >          self.assertEqual(asctime, "Sat Oct 22 9:06:04 2016")
> >
> > +    def test_delete(self):
> > +        self.cli_login()
> > +        self.add_project("QEMU", "qemu-devel@nongnu.org")
> > +        self.cli_import("0002-unusual-cased-tags.mbox.gz")
> > +        a, b = self.check_cli(["search", "-r", "-o",
> "subject,properties"])
> > +        ao = json.loads(a)[0]
> > +        self.assertEqual(["Fam Zheng", "famz@redhat.com"],
> > +                         ao["properties"]["reviewers"][0])
> > +        self.cli_delete("from:Fam")
> > +        a, b = self.check_cli(["search", "-r", "-o",
> "subject,properties"])
> > +        self.assertEqual("", a)
>
> While you are at it, please import another message that will not be
> deleted by
> the term, and verify it survives the detele command.
>
> Fam
>
> > +
> >  if __name__ == '__main__':
> >      main()
> > --
> > 2.15.1 (Apple Git-101)
> >
> > _______________________________________________
> > Patchew-devel mailing list
> > Patchew-devel@redhat.com
> > https://www.redhat.com/mailman/listinfo/patchew-devel
>
_______________________________________________
Patchew-devel mailing list
Patchew-devel@redhat.com
https://www.redhat.com/mailman/listinfo/patchew-devel
Re: [Patchew-devel] [PATCH v4 3/4] Make Delete Command use REST
Posted by Fam Zheng 6 years, 1 month ago
On Wed, 07/11 21:21, Shubham Jain wrote:
> Can we test this somehow?

Yeah, just 'time ./patchew-cli delete --all' on the sample DB.

Fam

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