Closed Bug 522249 Opened 11 years ago Closed 11 years ago

Add a way to export XML files.

Categories

(Webtools :: ISPDB Server, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0rc1

People

(Reporter: bwinton, Assigned: bwinton)

References

Details

Attachments

(3 files, 2 obsolete files)

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.
Flags: blocking-thunderbird3?
Priority: -- → P1
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.
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.)
Flags: blocking-thunderbird3? → blocking-thunderbird3-
Blocks: 526559
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.
Assignee: nobody → bwinton
Status: NEW → ASSIGNED
Attachment #413657 - Flags: review?(david.ascher)
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.
OS: Mac OS X → All
Hardware: x86 → All
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.
Attachment #413657 - Flags: review?(david.ascher) → review+
(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.
Right, as mentioned in comment #2, zip here is used as a packaging tool, not a compression tool.
(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.
(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.
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.
Attachment #414279 - Flags: review?(david.ascher)
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
Attachment #414279 - Flags: review?(david.ascher) → review+
And over to you for the second review, Gozer.

Thanks,
Blake.
Attachment #414279 - Attachment is obsolete: true
Attachment #414416 - Flags: review?(gozer)
(Uh, let's try that again, with the test case added this time.)
Attachment #414416 - Attachment is obsolete: true
Attachment #414418 - Flags: review?(gozer)
Attachment #414416 - Flags: review?(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.
Attachment #414418 - Flags: review?(gozer) → review+
Added, and checked in as
http://viewvc.svn.mozilla.org/vc?view=revision&revision=56849
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Component: ispdb → ISPDB Server
Product: Mozilla Messaging → Webtools
You need to log in before you can comment on or make changes to this bug.