Skip to content
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

Use try with resources in kubernetes-examples #2444

Closed
5 of 11 tasks
rohanKanojia opened this issue Aug 27, 2020 · 15 comments · Fixed by #2584
Closed
5 of 11 tasks

Use try with resources in kubernetes-examples #2444

rohanKanojia opened this issue Aug 27, 2020 · 15 comments · Fixed by #2584

Comments

@rohanKanojia
Copy link
Member

rohanKanojia commented Aug 27, 2020

We're not closing client in some of the examples after using it, we should either close it with client.close() or use KubernetesClient like this:

try (KubernetesClient client = new DefaultKubernetesClient()) {
  // Use Client for your stuff
}

Here are some places where it's not closed properly:

There are some more occurrences listed in Sonar Bug report. You can have a look at it here.

@Shivkumar13
Copy link
Contributor

/assign

@rohanKanojia
Copy link
Member Author

@Shivkumar13 : polite ping, Are you still working on this issue?

@Shivkumar13
Copy link
Contributor

@rohanKanojia apologies for the delay, I will update my findings in a day.
Was in other tasks for a while.

@HeroicHitesh
Copy link

@rohanKanojia Is this issue still relevant? If yes, I would like to work on it.

@rohanKanojia
Copy link
Member Author

Yes, it's still relevant. You might want to check with @Shivkumar13 if he is still working on it or not. There are lots of quickstarts that need to be updated, maybe you both can collaborate on refactoring these :-)

@HeroicHitesh
Copy link

@Shivkumar13 if you are working on it can you give an update on which all quickstart have you worked already and which all are still pending? so that I can start working on this issue.

@Shivkumar13
Copy link
Contributor

@HeroicHitesh I will raise a PR by tomorrow, need some cross checks

@Shivkumar13
Copy link
Contributor

I have the fix ready, while raising PR, seeing some old git changes also popping up in this Fix.
I will rebase the branch and will try again tomorrow.

@HeroicHitesh
Copy link

@Shivkumar13 any updates? You can send a PR for the files you have modified and I'll do the rest. Or if you are facing any issue related to Git/GitHub, let us know, maybe we can help you out

@Shivkumar13
Copy link
Contributor

@HeroicHitesh already raised a PR, day before yesterday

See: #2567

@HeroicHitesh
Copy link

@Shivkumar13 My bad I couldn't see any linked PR so thought you are stuck in that rebase thing. Now, from the PR I can see
https://github.com/fabric8io/kubernetes-client/blob/master/kubernetes-examples/src/main/java/io/fabric8/kubernetes/examples/LoadMultipleDocumentsFromFileExample.java

https://github.com/fabric8io/kubernetes-client/blob/master/kubernetes-examples/src/main/java/io/fabric8/openshift/examples/AdaptClient.java

these 2 are fixed and the remaining are pending. Do you have other fixes ready or can I start working on them?
Also, @rohanKanojia can you please make a checkbox in-front of the links in the issue description, so that it's easy to track which files are fixed and which are still pending.

@rohanKanojia
Copy link
Member Author

@HeroicHitesh : Thanks for pointing this out, I've updated the issue description.

@Shivkumar13
Copy link
Contributor

Shivkumar13 commented Oct 31, 2020

@Shivkumar13 My bad I couldn't see any linked PR so thought you are stuck in that rebase thing. Now, from the PR I can see
https://github.com/fabric8io/kubernetes-client/blob/master/kubernetes-examples/src/main/java/io/fabric8/kubernetes/examples/LoadMultipleDocumentsFromFileExample.java

https://github.com/fabric8io/kubernetes-client/blob/master/kubernetes-examples/src/main/java/io/fabric8/openshift/examples/AdaptClient.java

these 2 are fixed and the remaining are pending. Do you have other fixes ready or can I start working on them?

@HeroicHitesh I am not having other ready at the moment, but I can work on them, in the meantime, feel free to work on those.
please do list here the ones that you will be fixing. Then I can pick others.

Also, @rohanKanojia can you please make a checkbox in-front of the links in the issue description, so that it's easy to track which files are fixed and which are still pending.

@HeroicHitesh
Copy link

@rohanKanojia I have sent a PR for the following files:

  • ListCustomResourceDefinitions.java
  • BuildConfigExamples.java
  • LoadExample.java
  • DeploymentConfigExamples.java
    Please review and let me know what needs to be updated there.

@rohanKanojia
Copy link
Member Author

I think Marc updated all of the samples in #2688 . It's no longer relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants