You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by knusbaum <gi...@git.apache.org> on 2015/09/03 22:01:09 UTC

[GitHub] storm pull request: STORM-912: Support SSL on Logviewer

GitHub user knusbaum opened a pull request:

    https://github.com/apache/storm/pull/717

    STORM-912: Support SSL on Logviewer

    Backporting STORM-912 (#604) fix to 0.10.x branch. 

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

    $ git pull https://github.com/knusbaum/incubator-storm YSTORM-912_0.10

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

    https://github.com/apache/storm/pull/717.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 #717
    
----
commit 877143690aa42e90fe8d97e940090378d4ab2811
Author: Kyle Nusbaum <ky...@gmail.com>
Date:   2015-09-03T19:58:49Z

    Backporting STORM-912 fix to 0.10.x branch.

----


---
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: STORM-912: Support SSL on Logviewer

Posted by knusbaum <gi...@git.apache.org>.
Github user knusbaum commented on the pull request:

    https://github.com/apache/storm/pull/717#issuecomment-146656531
  
    @HeartSaVioR Sorry, it took me a while to get back to this. 
    
    The keyword is scheme, not schema. I've fixed it here, and I'll create a new pull request to master to fix it there, too.


---
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: STORM-912: Support SSL on Logviewer

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

    https://github.com/apache/storm/pull/717


---
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: STORM-912: Support SSL on Logviewer

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:

    https://github.com/apache/storm/pull/717#issuecomment-137578750
  
    Thinking it once more, we should maintain 0.10.x-branch for a fairly long time, so travis-ci should be fixed to future PRs.
    I addressed backporting #609 to #718. 


---
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: STORM-912: Support SSL on Logviewer

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:

    https://github.com/apache/storm/pull/717#issuecomment-146767039
  
    Great! +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: STORM-912: Support SSL on Logviewer

Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:

    https://github.com/apache/storm/pull/717#issuecomment-137575380
  
    > #609 resolves storm-hive calcite SNAPSHOT
    
    Thanks, @HeartSaVioR I was really confused when I saw it.


---
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: STORM-912: Support SSL on Logviewer

Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:

    https://github.com/apache/storm/pull/717#issuecomment-137571731
  
    +1 The travis-ci error was strange.  It was looking for calcite version 0.9.2-incubating-SNAPSHOT.  I built on OSX using the travis-ci command, and it built and passed all tests.


---
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: STORM-912: Support SSL on Logviewer

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:

    https://github.com/apache/storm/pull/717#issuecomment-137567610
  
    +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: STORM-912: Support SSL on Logviewer

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

    https://github.com/apache/storm/pull/717#discussion_r38888659
  
    --- Diff: storm-core/src/clj/backtype/storm/ui/core.clj ---
    @@ -951,17 +958,17 @@
       (GET "/api/v1/topology/summary" [:as {:keys [cookies servlet-request]} & m]
            (assert-authorized-user servlet-request "getClusterInfo")
            (json-response (all-topologies-summary) (:callback m)))
    -  (GET  "/api/v1/topology/:id" [:as {:keys [cookies servlet-request]} id & m]
    +  (GET  "/api/v1/topology/:id" [:as {:keys [cookies servlet-request schema]} id & m]
    --- End diff --
    
    @knusbaum 
    I don't know about compojure / ring, and I can't find related docs, but core.clj from master branch uses different key name.
    Actually master branch uses both schema and scheme.
    
    - route which uses schema as key: https://github.com/apache/storm/blob/master/storm-core/src/clj/backtype/storm/ui/core.clj#L997
    - route which uses scheme as key: https://github.com/apache/storm/blob/master/storm-core/src/clj/backtype/storm/ui/core.clj#L1004
    
    Which one is correct? Or both things are correct?


---
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: STORM-912: Support SSL on Logviewer

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:

    https://github.com/apache/storm/pull/717#issuecomment-141257754
  
    @knusbaum 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.
---

[GitHub] storm pull request: STORM-912: Support SSL on Logviewer

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:

    https://github.com/apache/storm/pull/717#issuecomment-137574155
  
    #609 resolves storm-hive calcite SNAPSHOT dependency issue, but I didn't backport it to 0.10.x-branch since it seems to be minor for users, but it is critical issue for travis-ci build.
    When we're willing to do pull request against 0.10.x-branch, we may be better to backport #609 to 0.10.x-branch, too.


---
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.
---