You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by zbdzzg <gi...@git.apache.org> on 2016/11/21 12:50:29 UTC

[GitHub] drill pull request #663: DRILL-5054: Provide source files of jquery-ui and j...

GitHub user zbdzzg opened a pull request:

    https://github.com/apache/drill/pull/663

    DRILL-5054: Provide source files of jquery-ui and jquery-dataTables locally

    Hi,
    
    Currently Drill uses CDN for serving source files of jquery-ui and jquery-dataTables. This is OK for most cases, but not working in an isolated environment.
    
    This is a patch adding these files so that Drill will work fine in intranet.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/zbdzzg/drill 5054

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/drill/pull/663.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #663
    
----
commit 55701b7494cf4b505f41a78179a4214090365bdf
Author: hongze.zhz <ho...@alibaba-inc.com>
Date:   2016-11-21T12:42:26Z

    DRILL-5054: Provide source files of jquery-ui and jquery-dataTables locally

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #663: DRILL-5054: Provide source files of jquery-ui and j...

Posted by zbdzzg <gi...@git.apache.org>.
Github user zbdzzg closed the pull request at:

    https://github.com/apache/drill/pull/663


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #663: DRILL-5054: Provide source files of jquery-ui and j...

Posted by parthchandra <gi...@git.apache.org>.
Github user parthchandra commented on a diff in the pull request:

    https://github.com/apache/drill/pull/663#discussion_r89419148
  
    --- Diff: exec/java-exec/src/main/resources/rest/query/result.ftl ---
    @@ -11,14 +11,13 @@
     
     <#include "*/generic.ftl">
     <#macro page_head>
    -  <link rel="stylesheet" type="text/css" href="//code.jquery.com/ui/1.10.3/themes/smoothness/jquery-ui.css">
    -  <link rel="stylesheet" type="text/css" href="//cdn.datatables.net/plug-ins/be7019ee387/integration/jqueryui/dataTables.jqueryui.css">
    +  <link rel="stylesheet" type="text/css" href="/static/css/jquery-ui.css">
    --- End diff --
    
    It's a good idea to have both CDN and local loading. Try CDN first and if that fails, try the local set. Typically, the browser would have already got jquery and other common js and css files cached and pages will therefore load faster. 
    See here: http://stackoverflow.com/questions/3832446/benefits-vs-pitfalls-of-hosting-jquery-locally
    and here: http://stackoverflow.com/questions/1014203/best-way-to-use-googles-hosted-jquery-but-fall-back-to-my-hosted-library-on-go
    
    Also, not too sure about including large js, png, and css files in the source tree. Should we just put in code to try to load from a local location, and provide instructions to users on how to download and set it up?
    
    If we do include the third party scripts, then I would also suggest coding the version number into the name we keep locally: jquery-1.10.2.min.js instead of jquery.min.js, dataTables.colVis-1.1.0.min.js instead of dataTables.colVis.min.js, etc.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request #663: DRILL-5054: Provide source files of jquery-ui and j...

Posted by zbdzzg <gi...@git.apache.org>.
Github user zbdzzg commented on a diff in the pull request:

    https://github.com/apache/drill/pull/663#discussion_r89448653
  
    --- Diff: exec/java-exec/src/main/resources/rest/query/result.ftl ---
    @@ -11,14 +11,13 @@
     
     <#include "*/generic.ftl">
     <#macro page_head>
    -  <link rel="stylesheet" type="text/css" href="//code.jquery.com/ui/1.10.3/themes/smoothness/jquery-ui.css">
    -  <link rel="stylesheet" type="text/css" href="//cdn.datatables.net/plug-ins/be7019ee387/integration/jqueryui/dataTables.jqueryui.css">
    +  <link rel="stylesheet" type="text/css" href="/static/css/jquery-ui.css">
    --- End diff --
    
    Hi Parthchandra,
    
    I have reconsidered this.
     
    Simply putting these things to source code is indeed not a graceful solution, even this method is just temporarily working in my case.
    
    I agree with your second advice: to make these stuffs downloadable and configurable. But as the typical scene for now is that users can already have them in cache, so may be just customizing and recompiling the code for special requirements like this is more reasonable for users like me (the scripts are not fetchable from my area, the network hangs while accessing cdn, util click "stop" actively. To have both the cdn and local files is therefore not work for me), I am so going to shelve this issue util someone report the same problem. Thank you.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---