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

Rationalize KubernetesList/BaseKubernetesList/DefaultKubernetesResourceList #3906

Closed
shawkins opened this issue Feb 26, 2022 · 4 comments · Fixed by #4499
Closed

Rationalize KubernetesList/BaseKubernetesList/DefaultKubernetesResourceList #3906

shawkins opened this issue Feb 26, 2022 · 4 comments · Fixed by #4499
Assignees
Milestone

Comments

@shawkins
Copy link
Contributor

shawkins commented Feb 26, 2022

Is your task related to a problem? Please describe

These classes after #3628 there are further cleanups that can be made to several list classes.

Describe the solution you'd like

  • BaseKubernetesList could can be merged with KubernetesList (this would mean changing/moving the generated class to be KubernetesList).
  • KubernetesList should implement KubernetesResourceList. - it already does
  • DefaultKubernetesResourceList and CustomResourceList would then be deprecated and indicate to use KubernetesList instead.

Describe alternatives you've considered

No response

Additional context

No response

@shawkins shawkins changed the title Deprecated/remove KubernetesList/BaseKubernetesList Rationalize KubernetesList/BaseKubernetesList/DefaultKubernetesResourceList Mar 1, 2022
@shawkins shawkins added this to the 6.0.0 milestone Mar 1, 2022
@shawkins shawkins self-assigned this Mar 9, 2022
@shawkins
Copy link
Contributor Author

shawkins commented Mar 10, 2022

Ideal state is:

  • deprecate/remove: DefaultKubernetesResourceList, CustomResourceList, and KubernetesResourceList
  • BaseKubernetesList becomes generic (and thus a replacement for CustomResourceList and DefaultKubernetesResourceList) and the generation logic is changed to make each resource list extend BaseKubernetesList
  • KubernetesList can still be used in the api where appropriate - if BaseKubernetesList<HasMetadata> is possible, you could even just rename BaseKubernetesList to KubernetesList to eliminate one more class.

Since the generation logic seems more involved that I want to mess with, that means we need to keep KubernetesResourceList as the resource lists will not have a common base with the other lists.

From there no matter what I try if I have a generic BaseKubernetesList something fails with the buildable logic with KubernetesList - if BaseKubernetesList is not buildable, the KubernetesListFluent isn't aware of the necessary fields. If BaseKubernetesList is buildable, I get:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.10.0:compile (default-compile) on project kubernetes-model-core: Fatal error compiling: java.lang.ClassCastException: class io.sundr.model.TypeParamRef cannot be cast to class io.sundr.model.ClassRef (io.sundr.model.TypeParamRef and io.sundr.model.ClassRef are in unnamed module of loader java.net.URLClassLoader @6921cfa)

So I think that means the I'm stuck. If we want to retain a KubernetesList builder, then we cannot improve on the current set of classes other than to deprecate CustomResourceList

@shawkins shawkins removed this from the 6.0.0 milestone Mar 10, 2022
@stale
Copy link

stale bot commented Jun 8, 2022

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

@stale stale bot added the status/stale label Jun 8, 2022
@manusa
Copy link
Member

manusa commented Jun 9, 2022

Shall we go ahead with the CustomResourceList deprecation?

@stale stale bot removed the status/stale label Jun 9, 2022
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Jun 24, 2022
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Jun 24, 2022
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Jun 29, 2022
@stale
Copy link

stale bot commented Sep 7, 2022

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

@stale stale bot added the status/stale label Sep 7, 2022
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Oct 14, 2022
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Oct 14, 2022
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Oct 14, 2022
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Oct 14, 2022
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Oct 14, 2022
shawkins added a commit to shawkins/kubernetes-client that referenced this issue Oct 14, 2022
manusa pushed a commit to shawkins/kubernetes-client that referenced this issue Oct 19, 2022
manusa pushed a commit to shawkins/kubernetes-client that referenced this issue Oct 19, 2022
@manusa manusa added this to the 6.2.0 milestone Oct 19, 2022
manusa pushed a commit that referenced this issue Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants