[GitHub] drill issue #891: DRILL-5699: Drill Web UI Page Source Has Links To External...
Github user parthchandra commented on the issue: https://github.com/apache/drill/pull/891 +1 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #891: DRILL-5699: Drill Web UI Page Source Has Links To External...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/891 @parthchandra could you please do final review? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #891: DRILL-5699: Drill Web UI Page Source Has Links To External...
Github user sindhurirayavaram commented on the issue: https://github.com/apache/drill/pull/891 @arina-ielchiieva After the first commit, I had a detailed discussion with @parthchandra. There are two reasons two include all the js and css files locally 1) The biggest library among them, the jquery is already included in the resources file. Even libraries like d3 are there locally. 2) One of the js and css libraries, Colvis by datatables is retired. We have to keep a local copy of that incase the url becomes extinct in the future. Check this please [Colvis](https://datatables.net/extensions/colvis/) So we decided to just include all the js and css libraries locally. I updated the libraries to their min versions and some of them to their latest versions. I made a change in generic page where we are loading google external cdn first and if its failing falling back to the local library. Please let me know if there are any changes needed here. 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #891: DRILL-5699: Drill Web UI Page Source Has Links To External...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/891 @sindhurirayavaram I am a little confused with your latest changes. I thought the main purpose of this PR is not to include js and css files into Drill code as was discussed in https://github.com/apache/drill/pull/663. I thought your first round of changes was correct, though we just needed to make sure, they work in different browsers, so basically I thought that all is left to do is to add `onload` so changes can be run in IE. Though maybe I am missing something ... --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #891: DRILL-5699: Drill Web UI Page Source Has Links To External...
Github user sindhurirayavaram commented on the issue: https://github.com/apache/drill/pull/891 @arina! I am planning to keep all the files in the local resources file! I have mentioned the reasons above. Yes, onload should be used for IE. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #891: DRILL-5699: Drill Web UI Page Source Has Links To External...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/891 @sindhurirayavaram, one more question about browser compatibility: will this work in Chrome, Mozilla, Safari etc? For example, quick search shows that there are can be issues with different browsers when using `link onerror` [1]. [1] https://stackoverflow.com/questions/30171270/link-onerror-do-not-work-in-ie --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #891: DRILL-5699: Drill Web UI Page Source Has Links To External...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/891 Regarding build failure, it will fail only if we did not download these files before. If we did, it will take them from the cache: `[INFO] --- download-maven-plugin:1.3.0:wget (add-datatables-css-as-resource) @ drill-java-exec --- [INFO] Got from cache: .m2\repository\.cache\download-maven-plugin\dataTables.colVis.css_050ee51b8fd84b79ae0240ce840501c6 ` Basically we need internet only when we build the project for the first time, the next time having internet access is not necessary, So I guess we are fine here. +1, LGTM. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #891: DRILL-5699: Drill Web UI Page Source Has Links To External...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/891 Actually you are right, there are benefits of using CDN first [1]. [1] https://stackoverflow.com/questions/1014203/best-way-to-use-googles-hosted-jquery-but-fall-back-to-my-hosted-library-on-go --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #891: DRILL-5699: Drill Web UI Page Source Has Links To External...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/891 > Even if we have local files it's better to check the browser's cache first. If that fails, we can load them locally. But why? javascript and css files have stable versions. If we download them during build time, they won't change after that. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #891: DRILL-5699: Drill Web UI Page Source Has Links To External...
Github user sindhurirayavaram commented on the issue: https://github.com/apache/drill/pull/891 @arina-ielchiieva Thanks for the comments! Yes drillbit fails if it can't download them during build time. These are pretty large fails. Is it good to add them in the source tree? To answer your second question! Even if we have local files it's better to check the browser's cache first. If that fails, we can load them 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #891: DRILL-5699: Drill Web UI Page Source Has Links To External...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/891 I have checked and build will fail: `Failed to execute goal com.googlecode.maven-download-plugin:download-maven-plugin:1.3.0:wget (add-jquery-as-resource) on project drill-java-exec: IO Error: Could not get content -> [Help 1]`, so I guess we just can refer to static resources. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #891: DRILL-5699: Drill Web UI Page Source Has Links To External...
Github user arina-ielchiieva commented on the issue: https://github.com/apache/drill/pull/891 @sindhurirayavaram, thanks for the PR! I have two questions though: 1. Will Drill build fail if we could not download these the javascript and css files during build time? 2. If the answer to the first question is no than why we try to download the javascript and css files from the internet if we have already loaded them locally when loading the result page? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
