Re: Logs of JobExecutionListener

[Theo Diefenthal <th...@scoop-software.de>]

I would vote for ClusterClient not being internal. I use it a lot in my end-to-end tests to e.g. trigger savepoints and shut down the streaming jobs which I think is not possible via ExecutionEnvironments. 

So in my opinion, having a more powerful ClusterClient adds a lot of powerful features for quite some usecases.. 

Best regards 
Theo 


Von: "Flavio Pompermaier" <po...@okkam.it> 
An: "Aljoscha Krettek" <al...@apache.org> 
CC: "user" <us...@flink.apache.org>, "Till Rohrmann" <tr...@apache.org> 
Gesendet: Mittwoch, 25. November 2020 10:44:57 
Betreff: Re: Logs of JobExecutionListener 

Thank you all for the clarification..now things are much more clear. I hope this discussion could be of clarification for other people having the same doubts. 

Best, 
Flavio 

On Wed, Nov 25, 2020 at 10:27 AM Aljoscha Krettek < [ mailto:aljoscha@apache.org | aljoscha@apache.org ] > wrote: 


One bigger problem here is that the code base is very inconsistent when 
it comes to the @Public//@Internal annotations. Initially, only the 
packages that were meant to be "public" had them. For example flink-core 
has thorough annotations. Packages that were not meant to have any 
user-facing code didn't initially have annotations, so flink-runtime has 
barely no annotations. 

The reason why some classes in non-public-facing packages have 
annotations is just that at some point someone decided to make something 
consciously @Public or @Internal. 

On 24.11.20 12:25, Flavio Pompermaier wrote: 
> ok that's fine to me, just add an @internal annotation on the 
> RestClusterClient if it is intended only for internal use.. but wouldn't be 
> easier to provide some sort of client generation facility (e.g. swagger or 
> similar)? 
> 
> Il mar 24 nov 2020, 11:38 Till Rohrmann < [ mailto:trohrmann@apache.org | trohrmann@apache.org ] > ha scritto: 
> 
>> I see the point in having a richer RestClusterClient. However, I think we 
>> first have to make a decision whether the RestClusterClient is something 
>> internal or not. If it is something internal, then only extending the 
>> RestClusterClient and not adding these convenience methods to ClusterClient 
>> could be quite easy. However if it is internal, then we don't need these 
>> methods because the RestClusterClient is not used in such a way that it is 
>> needed. I believe Flavio that you could build your own RestClusterClient 
>> offering these methods. 
>> 
>> If we say that these methods should be usable by users, then they should 
>> also be added to the ClusterClient. Here I see the problem that some of 
>> these methods won't work with a per-job or application deployment. For 
>> example, public JarUploadResponseBody uploadJar(Path uploadedFile) would 
>> not work. 
>> 
>> Long story short, I think the easiest solution would be to build yourself 
>> an utility class which offers the required methods. The second best option 
>> in my opinion would be to add these methods to the RestClusterClient w/o 
>> giving guarantees for their stability. 
>> 
>> Cheers, 
>> Till 
>> 
>> On Mon, Nov 23, 2020 at 8:29 PM Flavio Pompermaier < [ mailto:pompermaier@okkam.it | pompermaier@okkam.it ] > 
>> wrote: 
>> 
>>> For the sake of simplification (so everybody looking for missing methods 
>>> in RestClusterClient) I just shared the new methods at [1]. 
>>> In this way you can add them to the RestClusterClient when you want (if 
>>> you want to). 
>>> I also had to change the visibility of some variables and methods in 
>>> order to make it work. 
>>> Probably it would be useful to put DTOs of flink-webmonitor in a 
>>> standalone project in order to be "importable" in the client project.. 
>>> 
>>> Best, 
>>> Flavio 
>>> 
>>> [1] 
>>> [ https://github.com/fpompermaier/flink-job-server/blob/main/flink-rest-client/src/main/java/org/apache/flink/client/program/rest/RestClusterClientExtended.java | https://github.com/fpompermaier/flink-job-server/blob/main/flink-rest-client/src/main/java/org/apache/flink/client/program/rest/RestClusterClientExtended.java ] 
>>> 
>>> On Mon, Nov 23, 2020 at 4:38 PM Flavio Pompermaier < [ mailto:pompermaier@okkam.it | pompermaier@okkam.it ] > 
>>> wrote: 
>>> 
>>>> I don't know if they need to be added also to the ClusterClient but for 
>>>> sure they are missing in the RestClusterClient 
>>>> 
>>>> On Mon, Nov 23, 2020 at 4:31 PM Aljoscha Krettek < [ mailto:aljoscha@apache.org | aljoscha@apache.org ] > 
>>>> wrote: 
>>>> 
>>>>> On 23.11.20 16:26, Flavio Pompermaier wrote: 
>>>>>> Thank you Aljosha,.now that's more clear! 
>>>>>> I didn't know that jobGraph.getJobID() was the solution for my use 
>>>>> case..I 
>>>>>> was convinced that the job ID was assigned by the cluster! 
>>>>>> And to me it's really weird that the job listener was not called by 
>>>>> the 
>>>>>> submitJob...Probably this should be documented at least. 
>>>>>> In the meanwhile I extended a little bit the RestClusterClient..do you 
>>>>>> think it could be worth issuing a PR to add some unimplemented 
>>>>> methods? 
>>>>>> 
>>>>>> For example I added: 
>>>>>> - public JobExceptionsInfo getFlinkJobExceptionsInfo(JobID 
>>>>> flinkJobId); 
>>>>>> - public EmptyResponseBody deleteJar(String jarFileName); 
>>>>>> - public boolean isJobRunning(JobID fjid) 
>>>>>> - public JarUploadResponseBody uploadJar(Path uploadedFile); 
>>>>>> 
>>>>>> and I was also going to add jarRun.. 
>>>>> 
>>>>> I would be OK with adding these. But you would also need to add them to 
>>>>> the base ClusterClient, right? 
>>>>> 
>>>>> @Till or @Chesnay, any concerns with this? 
>>>> 
>>>> 
>