[GitHub] storm issue #2703: [STORM-3094] : Added topology name validation at client s...

2018-06-11 Thread ManoharVanam
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...

2018-06-07 Thread ManoharVanam
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...

2018-06-06 Thread srdo
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...

2018-06-06 Thread revans2
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 issue #2703: [STORM-3094] : Added topology name validation at client s...

2018-06-06 Thread danny0405
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...

2018-06-06 Thread revans2
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...

2018-06-05 Thread danny0405
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.


---