Skip to content

Merging changes from #229 and #231 - Two Solutions for Quarkus/Native Compilation #236

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

Closed
kirek007 opened this issue Nov 25, 2020 · 10 comments
Assignees

Comments

@kirek007
Copy link
Contributor

HI, We were discussing with @csviri how to merge changes introduced by #229 and #231 because merging #229 will require removing #231 almost completely. But on the other side we would like to avoid merging alpha version of kubernetes-client.

We came up with this kind of roadmap:

  1. Put feat: upgrade to k8s client 5.0 to simplify CR handling #229 on hold till release candidate of kubernetes-clinet (we agreed that this is minimal version that we can put in master)
  2. Merge Native Image Friendly #231
  3. Introduce quarkus-starter package that will add proper annotation for generated Doneable classes

But it's not made in stone. I've check and there is no public roadmap for kubernetes-client that would help us a lot to know when we can expect RC version. @metacosm if it will be ready soon, let's say in week or two then we could freeze both to finally merge only #229 because code generation won't be needed anymore and quarkus integration will be moved to dedicated package (if it will be needed).

I would love to hear your opinion about this @metacosm @psycho-ir @adam-sandor @csviri.

@csviri csviri changed the title Merging changes from #229 and #231 Merging changes from #229 and #231 - Two Solutions for Quarkus/Native Compilation Nov 25, 2020
@metacosm
Copy link
Collaborator

metacosm commented Nov 25, 2020

Doesn't #231 need to remove the quarkus dependency first, though? I mean native compilation could be achieved without it, it's just a matter of deciding whether this is worth the effort.

@kirek007
Copy link
Contributor Author

Indeed I've we have to remove java 8 parts and quarkus dependency

@metacosm
Copy link
Collaborator

The problem, though, is that #229 makes things a lot simpler and maintaining both branches will be a pain, especially considering that there are things that will need to be handled that will just go away if we were to use 5.0 because basically 5.0 was developed for the sole purpose of making things simpler for operator use cases.

@csviri
Copy link
Collaborator

csviri commented Nov 25, 2020

@metacosm Absolutelly, the final goal is to have it with client version 5.0, that would be the cleanest solution. The only concert here is the timeline. If the stable version of fabricate client will be released within few weeks, we should probably use that branch, and merge it as soon as the client is at least in RC version. (And discard the other one)
If you see that it might take longer, we could also go with an intermediary solution, by generating Doneable classes, until we have the client ready.
On the other hand if we merge now the client which is in alpha version, we will not be able to release further versions until the client is stable.

@metacosm What do you think, how long will it take a to release the new fabric8 client?
@psycho-ir What do you think about it?

@metacosm
Copy link
Collaborator

Maybe the best solution is to leave master in a stable state indeed and create a branch where all feature work would be performed with the expectation of using client 5.0 when it's ready? master would then only receive minimal changes in a stable fashion.

The problem with the fabric8 client 5.0 is that it's also experimental so we can indeed expect more changes that would be targeted at improving the operator-related experience. As such, we can't really release it until we get more feedback, feedback coming, for example, from being used in this project! So we're kind of in a deadlock situation where the sdk waits on the 5.0 release, while client 5.0 waits on feedback from project actually using it… 😄 My perspective on this is that both need to grow at the same time but that implies having a working sdk that uses 5.0 that people can actually use with the understanding that things are still evolving and bugs are expected.

@s-soroosh
Copy link
Contributor

One thing to note is that we also need to test the new version is compatible with [quarkus-openshift-client](https://github.com/java-operator-sdk/basic-native-operator-example/blob/init/pom.xml#L37.

@s-soroosh
Copy link
Contributor

@metacosm Absolutelly, the final goal is to have it with client version 5.0, that would be the cleanest solution. The only concert here is the timeline. If the stable version of fabricate client will be released within few weeks, we should probably use that branch, and merge it as soon as the client is at least in RC version. (And discard the other one)
If you see that it might take longer, we could also go with an intermediary solution, by generating Doneable classes, until we have the client ready.
On the other hand if we merge now the client which is in alpha version, we will not be able to release further versions until the client is stable.

@metacosm What do you think, how long will it take a to release the new fabric8 client?
@psycho-ir What do you think about it?

I think it makes sense to not rely on alpha dependencies in the mainstream but trying to simplify the APIs as much as possible (like the way we are going). IMO the developers will be very excited about the new features and the simplicity that the new version of the SDK has, Simpler annotations, Quarkus support, the universal eventing model to name a few.
I doubt depending their release to the release of Kubernetes-client RC is crucial!

@csviri
Copy link
Collaborator

csviri commented Nov 25, 2020

I agree with @psycho-ir , what we want to avoid is getting blocked to do releases.
We can still maintain the Fabric client alpha version support on a branch, test / play with it.

@adam-sandor
Copy link
Collaborator

I agree with the conclusions here. Merging #231 will enable native compilation until fabric8 5 is finalized, I think that's a good thing to have even temporarily. At the same time moving to alpha release of fabric8 is too risky and we might end up with bug reports caused by the underlying client. I think we can still provide very good feedback to the fabric8 team by implementing things on the branch of #229 and seeing if the new APIs work as intended.

@kirek007
Copy link
Contributor Author

Because situation is clear I'm going to close this issue and create dedicated branch for implementing kubernetes-client 5.0 se we can merge and close #299 for now. This will allow us to release test versions from this new branch if needed.

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

No branches or pull requests

5 participants