You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by Parth-Brahmbhatt <gi...@git.apache.org> on 2015/04/03 03:14:02 UTC
[GitHub] storm pull request: STORM-749: Remove CSRF check.
GitHub user Parth-Brahmbhatt opened a pull request:
https://github.com/apache/storm/pull/502
STORM-749: Remove CSRF check.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Parth-Brahmbhatt/incubator-storm STORM-749
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/502.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 #502
----
commit bebf11a25c23f0aa65a3cf5dc74ef02bb5cafeb6
Author: Parth Brahmbhatt <br...@gmail.com>
Date: 2015-04-03T01:13:18Z
STORM-749: Remove CSRF check.
----
---
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-749: Remove CSRF check.
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/storm/pull/502#issuecomment-89369393
Thanks. I am +1 for this when we have a similar pull request for the 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-749: Remove CSRF check.
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/502#discussion_r27732192
--- Diff: storm-core/src/clj/backtype/storm/ui/core.clj ---
@@ -975,7 +971,7 @@
name (.get_name tplg)]
(.activate nimbus name)
(log-message "Activating topology '" name "'")))
- (json-response (topology-op-response id "deactivate") (m "callback")))
+ (json-response (topology-op-response id "activate") (m "callback")))
--- End diff --
Good catch. This needs to be fixed in 0.10.x-branch also.
---
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-749: Remove CSRF check.
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/storm/pull/502#issuecomment-90618923
> I have a question. When we want to reflect PR into master and existing branch, is it encouraged to make two PRs pointing for each branches, or just one PR with cherry-pick?
I think we could cherry-pick, but either way you would have a new branch before the code is merged in.
1) check out 0.10.x-branch
2) create new branch
3) cherry-pick change (test, etc...)
4) make pull request to apache 0.10.x-branch
I do not think we should cherry-pick commits directly to a branch without a pull request, because it is a code change that should have a review.
@ptgoetz, any comment?
---
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-749: Remove CSRF check.
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/storm/pull/502#issuecomment-90677979
+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-749: Remove CSRF check.
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/storm/pull/502#issuecomment-89312662
> Maybe it would be better to remove "antiForgeryToken" from /api/v1/topology/:id's sample json.
Yes, STORM-UI-REST-API.md line 379.
---
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-749: Remove CSRF check.
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/storm/pull/502#issuecomment-89313418
https://github.com/Parth-Brahmbhatt/incubator-storm/blob/b7ed77727a1d1e76fc775ac6776b6580e25a1afe/storm-core/src/ui/public/topology.html#L125-L128
Should remove this too, since the template is gone.
---
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-749: Remove CSRF check.
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/502#issuecomment-89161672
LGTM.
Maybe it would be better to remove "antiForgeryToken" from /api/v1/topology/:id's sample json.
---
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-749: Remove CSRF check.
Posted by Parth-Brahmbhatt <gi...@git.apache.org>.
Github user Parth-Brahmbhatt commented on the pull request:
https://github.com/apache/storm/pull/502#issuecomment-90673501
Here is Pr against 0.10-x branch https://github.com/apache/storm/pull/512
---
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-749: Remove CSRF check.
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/502
---
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-749: Remove CSRF check.
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the pull request:
https://github.com/apache/storm/pull/502#issuecomment-89117111
+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-749: Remove CSRF check.
Posted by Parth-Brahmbhatt <gi...@git.apache.org>.
Github user Parth-Brahmbhatt commented on the pull request:
https://github.com/apache/storm/pull/502#issuecomment-89355652
@d2r @HeartSaVioR fixed.
---
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-749: Remove CSRF check.
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/502#issuecomment-90268685
I have a question. When we want to reflect PR into master and existing branch, is it encouraged to make two PRs pointing for each branches, or just one PR with cherry-pick?
---
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.
---