You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by abellina <gi...@git.apache.org> on 2016/07/01 19:33:45 UTC
[GitHub] storm pull request #1536: Storm 1890 ensure we refetch static resources afte...
GitHub user abellina opened a pull request:
https://github.com/apache/storm/pull/1536
Storm 1890 ensure we refetch static resources after package build
Original pull request: https://github.com/apache/storm/pull/1486
I added in this PR the no-cache header for html pages in the / directory. To recap, this is to add cache-busting to Storm UI.
I also upgraded ring-core to 1.3.1 and compojure to 1.1.9 to match ring-core versions without changing too much. Versions above 1.1.9 seem to have issues with Clout not compiling for clojure 1.7. We could potentially go to a much newer version which fixes the issues (1.4.0), but that should be a different PR IMHO, and I am not sure how much we care since this is going away on conversion to Java.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/abellina/storm STORM-1890_ensure_we_refetch_static_resources_after_package_build
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/1536.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 #1536
----
commit a76eadbdc01b8b484b40b4c8ef74753d2d68ca56
Author: Alessandro Bellina <ab...@yahoo-inc.com>
Date: 2016-06-14T12:39:45Z
STORM-1890: ensure static UI resources are re-requested with different Storm package builds
commit 42858abe3c2f13b0b6a5f997753d23ee149164ae
Author: Alessandro Bellina <ab...@yahoo-inc.com>
Date: 2016-06-18T19:04:36Z
STORM-1890: add temp-file store since wrap-multipart-params for ring 1.3 needs it
commit 9524679388564aa6c2b7dacf1737c2de8572dc6f
Author: Alessandro Bellina <ab...@yahoo-inc.com>
Date: 2016-06-18T19:05:27Z
STORM-1890: add code Cache-Control: no-cache to root html pages (/*.html)
commit 09c76f510a13a1a8f6454fd867f00579622fb488
Author: Alessandro Bellina <ab...@yahoo-inc.com>
Date: 2016-06-16T16:07:48Z
STORM-1890: add maven depency to for ring-core 1.3.1, move org.apache.commons.FileUtils to 1.3.2 as that is needed by ring-core 1.3.1, move compojure to 1.1.9 since it matches 1.3.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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #1536: Storm 1890 ensure we refetch static resources afte...
Posted by knusbaum <gi...@git.apache.org>.
Github user knusbaum commented on a diff in the pull request:
https://github.com/apache/storm/pull/1536#discussion_r70653301
--- Diff: storm-core/src/clj/org/apache/storm/ui/core.clj ---
@@ -18,7 +18,8 @@
(:use compojure.core)
(:use [clojure.java.shell :only [sh]])
(:use ring.middleware.reload
- ring.middleware.multipart-params)
+ ring.middleware.multipart-params
+ ring.middleware.multipart-params.temp-file)
--- End diff --
Nevermind. Shading makes this import necessary. Multipart-params tries to load it at runtime and blows up if it's not already in the namespace.
---
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] storm issue #1536: Storm 1890 ensure we refetch static resources after packa...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/1536
@abellina I don't have enough knowledge to front-end tech. But at a glance, I think concept is good.
@knusbaum @kishorvpatil Please go ahead reviewing & merging.
---
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] storm pull request #1536: Storm 1890 ensure we refetch static resources afte...
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on a diff in the pull request:
https://github.com/apache/storm/pull/1536#discussion_r70703286
--- Diff: storm-core/src/clj/org/apache/storm/ui/helpers.clj ---
@@ -36,22 +37,35 @@
[org.eclipse.jetty.server DispatcherType]
[org.eclipse.jetty.servlets CrossOriginFilter]
(org.json.simple JSONValue))
- (:require [ring.util servlet])
+ (:require [ring.util servlet]
+ [ring.util.response :as response])
(:require [compojure.route :as route]
[compojure.handler :as handler]))
;; TODO this function and its callings will be replace when ui.core and logviewer and drpc move to Java
(def num-web-requests (StormMetricsRegistry/registerMeter "num-web-requests"))
+
(defn requests-middleware
- "Coda Hale metric for counting the number of web requests."
+ "Wrap request with Coda Hale metric for counting the number of web requests,
+ and add Cache-Control: no-cache for html files in root directory (index.html, topology.html, etc)"
[handler]
(fn [req]
(.mark num-web-requests)
- (handler req)))
+ (let [uri (:uri req)
+ res (handler req)
+ content-type (response/get-header res "Content-Type")]
+ ;; check that the response is html and that the path is for a root page: a single / in the path
+ ;; then we know we don't want it cached (e.g. /index.html)
+ (if (and (= content-type "text/html")
+ (= 1 (StringUtils/countMatches uri "/")))
--- End diff --
```(count (re-seq #"/" url))``` could help find the # on "/" in more clojure like fashion.
---
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] storm issue #1536: Storm 1890 ensure we refetch static resources after packa...
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on the issue:
https://github.com/apache/storm/pull/1536
Thank you @abellina. It looks good. +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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1536: Storm 1890 ensure we refetch static resources after packa...
Posted by abellina <gi...@git.apache.org>.
Github user abellina commented on the issue:
https://github.com/apache/storm/pull/1536
@knusbaum, @kishorvpatil thanks for the review. @kishorvpatil I made the suggested change, thanks!
---
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] storm issue #1536: Storm 1890 ensure we refetch static resources after packa...
Posted by abellina <gi...@git.apache.org>.
Github user abellina commented on the issue:
https://github.com/apache/storm/pull/1536
@knusbaum @kishorvpatil @HeartSaVioR could you take a look at this PR? This will help with Storm UI caching issues in browsers. The build failures are unrelated.
---
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] storm issue #1536: Storm 1890 ensure we refetch static resources after packa...
Posted by knusbaum <gi...@git.apache.org>.
Github user knusbaum commented on the issue:
https://github.com/apache/storm/pull/1536
+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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #1536: Storm 1890 ensure we refetch static resources after packa...
Posted by abellina <gi...@git.apache.org>.
Github user abellina commented on the issue:
https://github.com/apache/storm/pull/1536
@HeartSaVioR, could you take a look at this PR?
---
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] storm issue #1536: Storm 1890 ensure we refetch static resources after packa...
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on the issue:
https://github.com/apache/storm/pull/1536
One minor comment. The rest looks good.
---
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] storm issue #1536: Storm 1890 ensure we refetch static resources after packa...
Posted by abellina <gi...@git.apache.org>.
Github user abellina commented on the issue:
https://github.com/apache/storm/pull/1536
No problem. Thanks @HeartSaVioR.
---
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] storm pull request #1536: Storm 1890 ensure we refetch static resources afte...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/1536
---
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] storm pull request #1536: Storm 1890 ensure we refetch static resources afte...
Posted by knusbaum <gi...@git.apache.org>.
Github user knusbaum commented on a diff in the pull request:
https://github.com/apache/storm/pull/1536#discussion_r70648943
--- Diff: storm-core/src/clj/org/apache/storm/ui/core.clj ---
@@ -18,7 +18,8 @@
(:use compojure.core)
(:use [clojure.java.shell :only [sh]])
(:use ring.middleware.reload
- ring.middleware.multipart-params)
+ ring.middleware.multipart-params
+ ring.middleware.multipart-params.temp-file)
--- End diff --
Is this a necessary import?
---
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] storm issue #1536: Storm 1890 ensure we refetch static resources after packa...
Posted by abellina <gi...@git.apache.org>.
Github user abellina commented on the issue:
https://github.com/apache/storm/pull/1536
ping
---
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.
---