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 2014/12/23 21:34:51 UTC
[GitHub] storm pull request: Storm-456: changing encoding to ring/url-encod...
GitHub user Parth-Brahmbhatt opened a pull request:
https://github.com/apache/storm/pull/360
Storm-456: changing encoding to ring/url-encode.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Parth-Brahmbhatt/incubator-storm Storm-456
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/360.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 #360
----
commit 16b7382dc7675d7a8b7aa0108af836913eef0560
Author: Parth Brahmbhatt <br...@gmail.com>
Date: 2014-12-23T19:56:08Z
STORM-456: Changing the url-encode method to use ring ecnode as ring auto decodes the requests with its own scheme.
commit d42d8dab3a7e5524f96ed1e9985381ca2936544e
Author: Parth Brahmbhatt <br...@gmail.com>
Date: 2014-12-23T20:19:58Z
Merge remote-tracking branch 'upstream/master' into Storm-456
----
---
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-456: changing encoding to ring/url-encod...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/360
---
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-456: changing encoding to ring/url-encod...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the pull request:
https://github.com/apache/storm/pull/360#issuecomment-69940108
+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-456: changing encoding to ring/url-encod...
Posted by Parth-Brahmbhatt <gi...@git.apache.org>.
Github user Parth-Brahmbhatt commented on the pull request:
https://github.com/apache/storm/pull/360#issuecomment-69600038
@harshach @revans2 made the suggested changes.
---
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-456: changing encoding to ring/url-encod...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the pull request:
https://github.com/apache/storm/pull/360#issuecomment-70344299
@Parth-Brahmbhatt Thanks for the patch , pushed into the master.
---
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-456: changing encoding to ring/url-encod...
Posted by Parth-Brahmbhatt <gi...@git.apache.org>.
Github user Parth-Brahmbhatt commented on the pull request:
https://github.com/apache/storm/pull/360#issuecomment-69466912
@revans2 I am not sure if it does not do the right thing, but our decoding in ui is always done by ring decoder and it definitely does not match with the encoding done by the java encoder. I like the idea of using the ring encoder everywhere so I made the changes.
---
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-456: changing encoding to ring/url-encod...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/360#issuecomment-69399742
For the most part this looks good and I am +1 on merging this in. However, I was wondering why we don't just replace the the backtype.storm.util url-encode with the ring version? or delete it and replace all of the calls to it everywhere. If it doesn't do the right thing we should stop using it everywhere.
---
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-456: changing encoding to ring/url-encod...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the pull request:
https://github.com/apache/storm/pull/360#issuecomment-69478368
@revans2 @Parth-Brahmbhatt I've the same question on url-encode. I think we should replace the backtype.storm.util url-encode with ring . The only place url-encode used other than ui is cluster.clj.
It's good to have one url-encode everywhere.
---
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.
---