-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactoring to remove some of the code around pumpers #3169
Conversation
Can one of the admins verify this patch? |
dd028a6
to
3d0d990
Compare
3d0d990
to
8a3b29d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I really like where this is going.
I added some comments/questions though about some things I would like to better understand.
...lient/src/main/java/io/fabric8/kubernetes/client/dsl/internal/core/v1/PodOperationsImpl.java
Show resolved
Hide resolved
...lient/src/main/java/io/fabric8/kubernetes/client/dsl/internal/core/v1/PodOperationsImpl.java
Show resolved
Hide resolved
kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/InputStreamPumper.java
Show resolved
Hide resolved
kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/Utils.java
Show resolved
Hide resolved
SonarCloud Quality Gate failed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/InputStreamPumper.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thx!
Description
I don't know if this is desirable, but with the other changes related to threading concerns the pumper classes and calls to utils.shutdownExecutorService seemed suspect. This change tries to separate out the various responsibilities of the pumpers to just a couple of static calls. I am not sure however if clients are using these classes (InputStreamPumper, Callback) as they ended up in the examples as well.
Type of change
test, version modification, documentation, etc.)
Checklist