[FLINK-2512]Add client.close() before throw RuntimeException

[ffbin]

In line 129, it close client in finally{} before throw exception.
But in line 105, it throw exception without close client.
So I think it is better to close client before throw RuntimeException.

[uce]

I think the best thing would be to just move the try up a little.

[ffbin]

@uce Thanks. What about remove "if(client.getTopologyJobId(name) != null) {...}" in line 103, because submitTopologyWithOpts() has check it at the head of function and will throw AlreadyAliveException.Then in finally{} close client.

[uce]

If you move the try up, you can certainly remove the manual close. Regarding the check in 103, it really depends on whether the Strom compat layer depends on having only a single job per client. Therefore I would keep it in and let it throw the RuntimeException as before. The finally block will then make sure that the client is closed.

[hsaputra]

Since getTopologyJobId can also throw exception, we could just move up the try-catch to include the call to that method and rely on finally to close the client.

[ffbin]

@uce @hsaputra Thanks. I have move the try up and rely on finally to close the client.

[StephanEwen]

Looks good, will merge this...

[hsaputra]

All builds are failing though, not sure if bc this patch

[hsaputra]

Shouldn't declaration of client be outside of the try clause?

[StephanEwen]

You are right, CI caught it. From my skimming over, the code looked good, but it does not compile...

[ffbin]

@hsaputra @StephanEwen Thanks. I have fix my code error.

[hsaputra]

+1