-
Notifications
You must be signed in to change notification settings - Fork 23
Update Fabric8 kubernetes client to 7.2.0 #69
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
Conversation
<scope>provided</scope> | ||
</dependency> | ||
<dependency> | ||
<groupId>io.sundr</groupId> |
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.
Why do we need this dependency? From the fabric8 release notes it seemed like the old api was based on Sundrio, but not the new one
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.
We are using sundrio annotations in the kafka-access code (to create builder classes e.g. for KafkaAccess).
We didn't need to add the dependency previously as it was coming in from crd-generator-apt, however as the new crd-generator-api-v2 does not include it, we must now add it to continue using those annotations in the kafka-access code
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.
I wonder if it would make sense to upgrade the JavaOperator SDK as well as that should have support for Fabric8 7.0.0 ideally as well.
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.
I wonder if we can get rid of the quotes in some way? Doesn't Fabric8 have some option for it?
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.
Not currently, but I've opened a PR to enable it - fabric8io/kubernetes-client#6773 so i guess can wait with this until I see how that goes
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.
Updated to 7.2.0 now instead as that contains the above PR to support not having the quotes
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.
Fabric8 7 switched the default HTTP client. So I think you need to update the exclude for the OkHttp client to VErt.x client?
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.
Done
@MichaelMorrisEst are you intending to pick this up again to address the comments? It looks like fabric8io/kubernetes-client#6773 got merged |
Hi @katheris, the change in the fabric8 kubernetes-client has been merged, but there has not been any release since. It is my intention to pick this up again when the kubernetes-client is released with the updates |
Including replacing the deprecated crd-generator-apt with its replacement crd-generator-api-v2 (See https://github.com/fabric8io/kubernetes-client/blob/main/doc/CRD-generator-migration-v2.md) Signed-off-by: MichaelMorris <michael.morris@est.tech>
Signed-off-by: MichaelMorris <michael.morris@est.tech>
0003343
to
d2dd422
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.
Thanks for PR @MichaelMorrisEst
Thanks for the PR. |
Update Fabric8 kubernetes client to 7.2.0
Including replacing the deprecated crd-generator-apt with its replacement crd-generator-api-v2 (See https://github.com/fabric8io/kubernetes-client/blob/main/doc/CRD-generator-migration-v2.md)
This should enable the completion of the remaining items of #53 (comment)