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 <trohrm...@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 <pomperma...@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
On Mon, Nov 23, 2020 at 4:38 PM Flavio Pompermaier <pomperma...@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 <aljos...@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?
