Change how jquery.js and jquery.dataTables.js dependencies are handled. - #2020

This seems ok in principle - are we ok with adding 0.6Mb to the tar file just for this feature? I personally would prefer to just use a configuration item that points to the online versions by default which could be changed by the user to a local version. IPython do this with mathjs and provide instructions on downloading it locally. Maybe we could follow that model?

That could work. I guess @olebole in that case would override those defaults to point to the Debian package-installed jquery. (There was discussion in #1992 that always hitting a CDN for these files represents a privacy violation).

Add jogging my own memory, the other issue with using online versions is that (obviously) they don't work offline and we would need to provide some sort of error detection and reporting when that downloading fails. The nice thing about this approach is that it's more likely to always work.

The error detection stuff is what IPython do. If you use ipython notebook offline and you need mathjs it gives instructions for what to do next time one is online in order to get it to work offline. I don't think jquery is really that critical at the moment, so I'd vote against including it in the repo and instead just giving an explicit error message if the internet is not on and local files are now found. What do others think?

👍 to everything @astrofrog says here. If we start finding more cases where jQuery is needed, and we get complaints from users on airplanes (well, I guess it's users after they get off the airplane...), then we can always stick it back in.

One more advantage to not including it in-source: we don't have to keep it up-to-date.

Oh, and this is probably obvious, but @olebole, it would be great to hear from you if this is an acceptable solution.

👍
If I didn't overlook anything, this looks like be the best solution for Debian: only real sources included, and I can simply ignore them :-)

@olebole - actually we are suggesting something different, which is to not include any sources but instead to fetch them from a configurable URL, which in the case of debian could be overridden to point to the local files. If a non-debian user tries to use the show_in_browser feature while not online, a verbose error message will give them instructions for how to set up jquery locally so that it works next time they are offline.

OK, I knew that I was missing something :-) However, not including the .js at all is fine for me as well.
What I still not completely understand: Are the .js files copied at installation time or at (first) usage?

@olebole - in the scenario I'm suggesting, they would only get used from the online versions by default and not copied. The user would need to manually copy them locally to get it to work.

To make it easy, we could provide a way for the user to do:

>>> from astropy.utils import cache_javascript
>>> cache_javascript()

This would download any remote javascript we depend on to the astropy cache and update any configuration (or alternatively the default for astropy would be to first check the local cache folder for the javascript before getting it online).

Then if someone tries to do the show_in_browser while offline, it will say:

>>> t.show_in_browser()
...
MissingJavascriptError: you appear to be currently offline, and do not have a local version
of the javascript files required for ``show_in_browser``. To download a local copy of the
jquery library, next time you are online, do:

    >>> from astropy.utils import cache_javascript
    >>> cache_javascript()

You will then be able to make use of this feature while offline in future.

I don't see the big deal. I think the solution in this PR is fine (how is it adding 0.6MB though? jQuery itself is not even that big uncompressed...)

ETA: turns out dataTables is a lot larger than I would have expected. Most of it seems to be docstrings though. Is it a problem if we edit it to remove the docstrings? If size is an issue (which I don't think it should be...)

@embray - I'm not sure I'm comfortable altering the source at all, because then someone will have to do that every time we decide we want a new version. That's a big part of why I like @astrofrog's idea.

@eteq: I don't follow: What source is being altered?

@mdboom: That was in reference to @embray's suggestion of removing the docstrings to shrink down the dataTables source. Strictly speaking, it's not actually removing the source code, but I just mean it involves editing the file to do this everytime we want to get a newer version.

(to clarify, as this is a bit convoluted, I like @astrofrog's most recent proposed solution of having a configuration item that defaults to hitting the CDN, but that packagers could point to their own package, or users can explicitly point to wherever they like)

I think we're having trouble reaching consensus here. Let me summarize:

Here's the things we need to do to make the Debian packaging easier:

1. not including any minimized source code without also including the original source code (including just the original source code is also fine), though Debian is likely to remove all that anyway in favor of using their own jquery package.

2. Minimizing the size and complexity of the Debian patch. I think having the Debian patch change a remote URL to a local file path is fine, but much beyond that is asking for too much coordination effort.

3. Debian doesn't yet have jquery.datatables packaged. So in the meantime, if Debian's policy doesn't let us fetch from CDN, we need to include a local copy in our source tree, or have Debian package it or add it to our package. See Created improved version of workflow documents based on gitwash documents #2 for why the last two options aren't great.

Then there's another goal for non-Debian installs, which is debatable:

4. Allow this all to work without an Internet connection

If we include the files (as here), we meet all of the criteria, at the cost of 0.6MB of installed data.

If we don't include the files, we lose #4 (but maybe we don't care), but we also screw up #3 (until datatables is packaged).

Maybe @olebole has a sense of the timeline on getting jquery.datatables in Debian, which may tip the scales in one direction or another.

libjs-jquery-datatable is on its way (waiting for the sponsor to upload, see https://bugs.debian.org/719601), so I would "recommend:" jquery-datatable and jquery in the package and disable any automated download.

@mdboom @olebole - here's a possible solution to make everyone happy:

• Modify this patch to not include any of the javascript files, but instead to use a remote version at runtime set by a config option (most seemed to agree with that above)
• Make it so the functionality raises an exception if jquery and dataTables cannot be fetched
• Debian is free to include a local version if they want and adjust the configuration option.

What would be the issues with this?

(edit: jquery source -> all javascript source files)

Fine for me, however it would save me a patch if the config option would also allow to specify a locally installed file.

file:/// would do it, shouldn't it? Anyways, I'd be happy with that solution.

Would work for me. I would, however, need that the file is not copied, but used from its installed place (so that updates for this file may come when the jquery package is updated).

Is this ready to go?

@embray - I don't think so - I think the consensus seems to be to not include the javascript files here (see my previous comment)

Uh remind me again why we would remove the JQuery source? It seems especially odd that we would ship a copy of the dataTables library but not include JQuery as well. If the minimized version is really problematic then just don't include the minified version. It doesn't provide any particular benefit for running locally anyway.

@embray - I'm actually saying we don't ship any of the javascript, i.e. jquery and dataTables. I had mentioned jquery in my previous comment because I thought dataTables was an extension to jquery, but I'll rephrase. I just don't see the value in including 600k of javascript in the astropy source just for a single feature, so that's my objection to including any of it locally.

I've amended my suggestion in #2020 (comment)

I guess I don't see 600k of source code as a big deal, but maybe that's just me?

When the astropy source tarball is already 23MB uncompressed, what's another 600k?

Wow, I'm surprised it's even that much. Also I can't tell if that was sarcasm or not :) But yeah what is another 600k?

Unless anyone stops me (@astrofrog?) I'm going ahead and merging this for 0.3.2 so that we don't have to keep @olebole waiting on it any longer. If anyone really starts to notice the larger archive size we can revisit.

@embray - ok, sounds good to me. I didn't realize the source tarball was so big!