Add an initial logs utility class. #87
Conversation
| if (!response.isSuccessful()) { | ||
| throw new ApiException("Logs request failed: " + response.code()); | ||
| } | ||
| return response.body().byteStream(); |
There was a problem hiding this comment.
Do we always know what this content-type will be? (for decoding)
There was a problem hiding this comment.
Added JavaDoc (it's text/plain)
|
|
||
| // Important note. You must close this stream or else you can leak connections. | ||
| public InputStream streamNamespacedPodLog(String namespace, String name, String container, | ||
| Integer sinceSeconds, Integer tailLines, boolean timestamps) throws ApiException, IOException { |
There was a problem hiding this comment.
Is sinceSeconds since epoch, or when the container started?
There was a problem hiding this comment.
Added JavaDoc (stolen from Swagger docs)
| * Utility class offering streaming access to Pod logs. | ||
| */ | ||
| public class Logs { | ||
| private ApiClient apiClient; |
There was a problem hiding this comment.
suggestion: I would make apiClient public final and remove setter and getter for it.
There was a problem hiding this comment.
That means you don't need a getter for it. It is a common practice I guess with pros and cons. But now that I think more about it, maybe having getter is not a bad idea. But I still suggest we remove setter for this and make it private final.
There was a problem hiding this comment.
Yeah was just curious since the rest of the generated code uses getters in favor of public final. Agreed that a setter probably shouldn't exist.
There was a problem hiding this comment.
Removed the setter.
| * @param pod The Pod to get logs for, uses the first container in the pod for logs | ||
| * @return A UTF-8 encoded InputStream | ||
| */ | ||
| public InputStream streamNamespacedPodLog(V1Pod pod) throws ApiException, IOException { |
There was a problem hiding this comment.
should we just call this streamPodLog?
There was a problem hiding this comment.
I think this is done for consistency, all the generated api methods seem to follow <verb>Namespaced<resource> for this kind of stuff
There was a problem hiding this comment.
Yeah, this was for consistency...
| /** | ||
| * Utility class offering streaming access to Pod logs. | ||
| */ | ||
| public class Logs { |
There was a problem hiding this comment.
Would this be confused with a Logger? When I saw the topic for this PR, I though this is a special logger. I suggest we call it something like PodLogs?
|
Comments addressed, please re-check. Thanks! |
|
/LGTM |
Adds capability to get logs as a stream, not just as a string.