Add Watch Support #40
Conversation
| System.out.printf("%s : %s%n", item.type, item.object.getMetadata().getName()); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
nit: newline at end of file?
(I actually don't care but I feel like you asked for this in a different PR and we should probably be consistent...)
There was a problem hiding this comment.
Yep, I think we should be consistent. Done.
| throw new ApiException(response.message(), response.code(), response.headers().toMultimap(), respBody); | ||
| } | ||
| return new Watch<>(this.json, response.body(), watchType); | ||
| } catch (IOException e) { |
There was a problem hiding this comment.
Why not have this throw IOException also? Seems better than catching and wrapping...
There was a problem hiding this comment.
I just wanted to be consistent with the rest of the ApiClient code. Execute method, for example, do the same (as well as other methods).
| * set watch to True and watch the changes to namespaces. | ||
| */ | ||
| public class Watch<T> implements Iterable<Watch.Response<T>> { | ||
|
|
| public boolean hasNext() { | ||
| if (loadNext) { | ||
| try { | ||
| nextItem = Watch.this.next(); |
There was a problem hiding this comment.
Hrm, I don't think that this obeys the Java iterator contract.
There is no requirement that a user call hasNext() to advance the iterator afaik. Why not just have all of this logic in the next() method?
There was a problem hiding this comment.
I am calling hasNext() in next() method so there is no need for user to call this before next(). I faced two options here, to raise exception on next() or hasNext(). I chose this way so when I use for-each with this iterator and the watch closes because of timeout, it won't raise an exception (because for-each calls hasNext() before next()). Of course if user cares about what happen they can always call next() and get the reason (or just call next() on Watch class itself).
There was a problem hiding this comment.
I've looked at this at another angle and made is much much simpler now. Please look at the Watch class as a whole again. It is implements both Iterable and Iterator and converts all IOExceptions to RuntimeException to be able to implement Iterator.
|
Comments addressed or replied to. Please take another look. |
|
LGTM. |
This is my first try to implement watch. It is mostly implemented like the
Watchin python client. We can think of changing templates (for both python and java) to create awatchcall for eachlistoperation in API classes.Fixes #8
depends on kubernetes-client/gen#16
built on top of #39