You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by ManoharVanam <gi...@git.apache.org> on 2018/06/05 18:41:27 UTC
[GitHub] storm pull request #2703: [STORM-3094] : Added topology name validation at c...
GitHub user ManoharVanam opened a pull request:
https://github.com/apache/storm/pull/2703
[STORM-3094] : Added topology name validation at client side
Current Behavior : Execute topology with invalid topology name is throwing exception after uploading the jar.
Improvement : Validating topology name at client side before uploading the jar.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ManoharVanam/storm STORM-3094
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/2703.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 #2703
----
commit 45a8f3218c96738af075e7e984239a88b34eff01
Author: Manohar Vanam <ma...@...>
Date: 2018-06-05T18:37:30Z
[STORM-3094] : Added topology name validation at client side
----
---
[GitHub] storm issue #2703: [STORM-3094] : Added topology name validation at client s...
Posted by ManoharVanam <gi...@git.apache.org>.
Github user ManoharVanam commented on the issue:
https://github.com/apache/storm/pull/2703
@HeartSaVioR done.
---
[GitHub] storm issue #2703: [STORM-3094] : Added topology name validation at client s...
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2703
+1, it seems fine to me to duplicate the check for convenience
---
[GitHub] storm issue #2703: [STORM-3094] : Added topology name validation at client s...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/2703
@danny0405 This is just a performance/usability optimization. The checks are still happening on the server side, so the other clients will still get an exception when they try to upload a jar, just like they did before, but the java client will not even try to upload the jar. It will fail faster.
---
[GitHub] storm issue #2703: [STORM-3094] : Added topology name validation at client s...
Posted by danny0405 <gi...@git.apache.org>.
Github user danny0405 commented on the issue:
https://github.com/apache/storm/pull/2703
Sorry i don't think put the validation at Client side is a good idea, cause many kind of clients may interact with Master through thrift RPC, we must keep all the clients sync on this validation rule if we moved it to client side.
Now the client kind i know:
- java script
- java
- python
Even if we all modified and sync the rule between all the client, we still can not prevent some private modification to break it.
---
[GitHub] storm issue #2703: [STORM-3094] : Added topology name validation at client s...
Posted by danny0405 <gi...@git.apache.org>.
Github user danny0405 commented on the issue:
https://github.com/apache/storm/pull/2703
@revans2
You are right, i just thought if we can keep all these clients sync in rule, but fail fast just for this is ok.
---
[GitHub] storm issue #2703: [STORM-3094] : Added topology name validation at client s...
Posted by ManoharVanam <gi...@git.apache.org>.
Github user ManoharVanam commented on the issue:
https://github.com/apache/storm/pull/2703
Yes , here we are handling two things,
1. Fail fast without uploading the jar
2. Giving clear message to the user at the client side.
---
[GitHub] storm issue #2703: [STORM-3094] : Added topology name validation at client s...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/2703
I agree it would be great to keep all of the clients in sync, but the java is the only high level client that we support. The only other client we generate the code for is a python client, but it is just the generated thrift code, not a high level API like for storm submitter.
---
[GitHub] storm pull request #2703: [STORM-3094] : Added topology name validation at c...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2703
---