522249 - Add a way to export XML files.

Status: RESOLVED FIXED

(Webtools :: ISPDB Server, defect, P1)

Description

[Blake Winton (:bwinton) (:☕️)]

In order to get the domains that users add from the ISPDB database to a place where Thunderbird will pick them up, we will need to be able to export all the confirmed domains into a set of XML files which can then be transferred over to the autoconfig.mozillamessaging.com svn repo.

The original suggestion would be to have a script do the conversion, but I think we might be able to have a url that returns a zip file containing all the xml files for the domains.

Comment 1

[David Ascher (:davida)]

Note: we already have a view:

    url(r'^export_xml/(?P<id>\d*)', 'ispdb.config.views.export_xml', name='ispdb_export_xml'),

to do it per id.  

I'd talk to gozer, but I suspect there's no real benefit to zipping the data ourselves.  We can generate HTTP headers if we want apache to compress the data over the wire, but I don't think there's a demonstrated need.

I agree that it should be done in the django views, not in an external script.  An external script can then call that URL.

Comment 2

[Blake Winton (:bwinton) (:☕️)]

Sure, I was thinking of the zip as more of a collection format, instead of a compression format.  (i.e. if we had a url that returned a tar file of xml configs, I would be just as happy.)

Comment 3

[Blake Winton (:bwinton) (:☕️)]

Attachment: A patch to export all the approved XML files, with tests.

With this patch, Gozer can hit a url, get all the approved configs in a zip, throw them into the autoconfig.mozillamessaging.com folder, and see what's changed with an svn diff.

Thanks,
Blake.

Comment 4

[Blake Winton (:bwinton) (:☕️)]

Gozer,

I would love it if you could test this change out, and make sure you can open the zip file on your favourite platform.  I've had a surprising number of problems with zip files transferring between Linux, OS X, and Windows.

Comment 5

[David Ascher (:davida)]

Comment on attachment 413657 [details] [diff] [review]
A patch to export all the approved XML files, with tests.

>Index: tests/test_export.py

>+    def test_empty_reponse(self):
>+        response = self.client.post(reverse("ispdb_export_xml"),{})
>+        # set_trace()

debug comment?

>+    def test_single_reponse(self):
>+        response = self.client.post(reverse("ispdb_add"), ExportTest.test2dict)
>+        assert_equal(response.status_code,302)
>+        domain = models.Domain.objects.get(name="test2.com")
>+        assert isinstance(domain,models.Domain)
>+
>+        response = self.client.post(reverse("ispdb_approve", kwargs={"id":domain.id}),
>+                                {"approved":"mark valid",
>+                                 "comment":"Always enter a comment here"})
>+        assert_equal(response.status_code, 302)
>+
>+        response = self.client.post(reverse("ispdb_export_xml"),{})
>+        assert_equal(response.status_code, 200)
>+        assert_equal(response["Content-Type"], "application/zip")
>+        assert_equal(response["Content-Disposition"], "filename=configs.zip")
>+        assert_not_equal(len(response.content), 0)
>+        zip = zipfile.ZipFile(StringIO(response.content), "r")
>+        #set_trace()

debug comment

>+        configs = zip.infolist()
>+        assert_equal(len(configs), 1)
>+        assert_equal(configs[0].filename, "test2.com")
>+        filedata = zip.read(configs[0].filename)
>+        filedata = re.sub("Datetime>.*</", "Datetime></", filedata)

I assume that's to deal with the variability in the output due to timestamping, but I'd add a comment. (same for the same thing in test_two_responses)

>Index: config/views.py
>===================================================================
>--- config/views.py	(revision 54976)
>+++ config/views.py	(working copy)
...
>-def export_xml(request, id):
>+def export_xml(request, id=None):
>     from django.core import serializers
>-    config = Config.objects.filter(id=int(id))[0]
>-    data = config.as_xml()
>-    return HttpResponse(data, mimetype='text/xml')
>+    if id:

I would suggest

   if id is not None:

to explictely limit this to the default argument handler case.

>+      config = Config.objects.filter(id=int(id))[0]
>+      data = config.as_xml()
>+      return HttpResponse(data, mimetype='text/xml')
>+    else:
>+      configs = Config.objects.all()
>+      output = StringIO()
>+      zip = zipfile.ZipFile(output, "w")
>+      for config in configs:
>+        print config.domains.all()[0], config.approved

not sure if we want or don't want the stdout output here.  

>+        if not config.approved:
>+          continue
>+        for domain in config.domains.all():
>+          zip.writestr(domain.name, config.as_xml())
>+      zip.close()
>+      output.flush()
>+      response = HttpResponse(mimetype="application/zip")
>+      response['Content-Disposition'] = 'filename=configs.zip'
>+      response.write(output.getvalue())
>+      output.close()
>+      return response

That's nice.


r=davida with those minor nits fixed.

Comment 6

[Philippe M. Chiasson (:gozer)]

(In reply to comment #1)
> Note: we already have a view:
> 
>     url(r'^export_xml/(?P<id>\d*)', 'ispdb.config.views.export_xml',
> name='ispdb_export_xml'),
> 
> to do it per id.  
> 
> I'd talk to gozer, but I suspect there's no real benefit to zipping the data
> ourselves. 

Not for general transport, nope. There is the excellent mod_deflate for that.

> We can generate HTTP headers if we want apache to compress the data
> over the wire, but I don't think there's a demonstrated need.

Not even a need to set any special headers, it's all handled up-front, an app like ispdb has no clue it's hapenning.

Comment 7

[David Ascher (:davida)]

Right, as mentioned in comment #2, zip here is used as a packaging tool, not a compression tool.

Comment 8

[Philippe M. Chiasson (:gozer)]

(In reply to comment #2)
> Sure, I was thinking of the zip as more of a collection format, instead of a
> compression format.  (i.e. if we had a url that returned a tar file of xml
> configs, I would be just as happy.)

I would actually prefer keeping the server-side of things as simple as possible.

For something like this, I'd rather see a script that:
 1 - Asks the ISPDB for a list of ISPs
 2 - Fetches the config of each ISP via the existing export_xml stuff

2 can be quite smart client-side, and perform local caching, conditional if-modified gets, and more to keep the svn checkout in sync.

In my experience, it's always easier to manage a simpler server-side and a possibly complex client, rather than the other way around.

Comment 9

[Blake Winton (:bwinton) (:☕️)]

Attachment: A list of config entry ids, with times.

(In reply to comment #8)
> > Sure, I was thinking of the zip as more of a collection format, instead of a
> > compression format.  (i.e. if we had a url that returned a tar file of xml
> > configs, I would be just as happy.)
> I would actually prefer keeping the server-side of things as simple as
> possible.

Neither of these options were that complicated.  :)

> For something like this, I'd rather see a script that:
>  1 - Asks the ISPDB for a list of ISPs

So, I've got a new patch that will let you hit http://ispdb.mozillamessaging.com/list/xml and get back a file like the one I attached.

>  2 - Fetches the config of each ISP via the existing export_xml stuff

We can do this with http://ispdb.mozillamessaging.com/export_xml/<id from the attached file>

Does this look more like something you'ld use, Gozer?
Is there anything else you'ld like to see in the data?
Would you rather JSON, XML, or something else?

Thanks,
Blake.

Comment 10

[Blake Winton (:bwinton) (:☕️)]

Attachment: A patch to add an xml format to the list view, with tests.

I think this should be closer to what Gozer wants.  (I'll ask him for the second review, though, to make sure.)

I've also added some small tests for the list view itself, cause I was in the code.

Thanks,
Blake.

Comment 11

[David Ascher (:davida)]

Comment on attachment 414279 [details] [diff] [review]
A patch to add an xml format to the list view, with tests.

>+def get_config(value):

I'd suggest make_config() or generate_config() -- reading the client of this function I expected it to fetch a config by name, not create one with an id.



r=davida

Comment 12

[Blake Winton (:bwinton) (:☕️)]

Attachment: The previous patch, with davida's nits fixed.

And over to you for the second review, Gozer.

Thanks,
Blake.

Comment 13

[Blake Winton (:bwinton) (:☕️)]

Attachment: The previous patch, with davida's nits fixed.

(Uh, let's try that again, with the test case added this time.)

Comment 14

[Philippe M. Chiasson (:gozer)]

Comment on attachment 414418 [details] [diff] [review]
The previous patch, with davida's nits fixed.

Look great, only thing I'd add is an extra 'export' field to each provider entry that contains a full url to the /export_xml/[id] view. Makes discovery much easier.

Comment 15

[Blake Winton (:bwinton) (:☕️)]

Added, and checked in as
http://viewvc.svn.mozilla.org/vc?view=revision&revision=56849