(DEPRECATED) Apache Flink User Mailing List archive.

Logs of JobExecutionListener

Classic

List

Threaded

28 messages Options

Aljoscha Krettek

Re: Logs of JobExecutionListener

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?

Flavio Pompermaier

Re: Logs of JobExecutionListener

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 <[hidden email]> 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?

Flavio Pompermaier

Re: Logs of JobExecutionListener

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

On Mon, Nov 23, 2020 at 4:38 PM Flavio Pompermaier <[hidden email]> 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 <[hidden email]> 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?

Till Rohrmann

Re: Logs of JobExecutionListener

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 <[hidden email]> 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

On Mon, Nov 23, 2020 at 4:38 PM Flavio Pompermaier <[hidden email]> 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 <[hidden email]> 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?

Flavio Pompermaier

Re: Logs of JobExecutionListener

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 <[hidden email]> 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 <[hidden email]> 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

On Mon, Nov 23, 2020 at 4:38 PM Flavio Pompermaier <[hidden email]> 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 <[hidden email]> 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?

Aljoscha Krettek

Re: Logs of JobExecutionListener

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 <[hidden email]> 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 <[hidden email]>
>> 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
>>>
>>> On Mon, Nov 23, 2020 at 4:38 PM Flavio Pompermaier <[hidden email]>
>>> 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 <[hidden email]>
>>>> 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?
>>>>
>>>>
>

Flavio Pompermaier

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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>
>> 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
>>>
>>> On Mon, Nov 23, 2020 at 4:38 PM Flavio Pompermaier <[hidden email]>
>>> 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 <[hidden email]>
>>>> 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?
>>>>
>>>>
>

Theo

Re: Logs of JobExecutionListener

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" <[hidden email]>
An: "Aljoscha Krettek" <[hidden email]>
CC: "user" <[hidden email]>, "Till Rohrmann" <[hidden email]>
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>
>> 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
>>>
>>> On Mon, Nov 23, 2020 at 4:38 PM Flavio Pompermaier <[hidden email]>
>>> 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 <[hidden email]>
>>>> 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?
>>>>
>>>>
>