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

[proposal] Publish a generated extension for Java #3994

Merged
merged 3 commits into from
Jan 26, 2023

Conversation

andreaTP
Copy link
Contributor

@andreaTP andreaTP commented Jan 19, 2023

Hi all and thanks for the amazing and widely used project!

For some time we are delivering a java-generator from CRD as part of the fabric8 kubernetes-client project.
We are dissatisfied with the current "extensions" mechanism and, sometimes, we struggle to keep it up to date and maintained.

Here I'm proposing a possible implementation of the automation needed in this repository to publish a package functionally equivalent to the current extension but with close to zero maintenance involved.

You can check the resulting artifacts built from my fork: https://jitpack.io/#andreaTP/camel-k/0.0.2-SNAPSHOT

I'm happy to receive feedback on all the pieces, the automation(jbang and jitpack), the generated code, where to publish etc.

cc. @lburgazzoli @manusa

Release Note

feat: Publish a generated extension for Java

@squakez
Copy link
Contributor

squakez commented Jan 20, 2023

Thanks for submitting the PR. However I am not able to entirely understand how this is going to work. Is it some user that need to run some command with the proposed configuration? Can you please provide a step by step description of the process? Thanks.

Copy link
Contributor

@oscerd oscerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this would go in substitution of the crd extension for camel-k coming from the Kubernetes client project?

@andreaTP
Copy link
Contributor Author

@squakez Sure, sorry for the missing context, here the ultimate goal is to provide the users a Java library to easily interact with typed Camel-K Custom Resources using the fabric8 kubernetes-client.
You can test the result of this PR using e.g. jbang and you will list the CamelCatalogs in a cluster with a script like:

//REPOS mavencentral,jitpack
//DEPS io.fabric8:kubernetes-client:6.4.0
//DEPS io.fabric8:generator-annotations:6.4.0
//DEPS com.github.andreaTP:camel-k:0.0.2-SNAPSHOT

import io.fabric8.kubernetes.client.*;
import org.apache.camel.v1.*;
public class testCamelKExtension {
    public static void main(String... args) {
        var client = new KubernetesClientBuilder().build();
        var camelCatalogClient = client.resources(CamelCatalog.class);
        camelCatalogClient.list().getItems().forEach(c -> {
            System.out.println(c.getMetadata().getName());
        });
    }
}

This is a user experience very close to the one offered by the current extension, let me know if you have any additional doubt or question, happy to clarify 🙂 .

So this would go in substitution of the crd extension for camel-k coming from the Kubernetes client project?

@oscerd this is completely up for discussion, I'm proposing this alternative approach and we can think about keeping the 2 available for some time to see how this works out, wdyt?

@squakez
Copy link
Contributor

squakez commented Jan 20, 2023

What I still miss to understand who is going to consume such new configurations and how the jar will end up into com.github.andreaTP:camel-k:0.0.2-SNAPSHOT. I suppose you have some process that will use this configuration for publishing the artifact, is it so?

@andreaTP
Copy link
Contributor Author

who is going to consume such new configurations

Here we are publishing standard Java Artifacts (i.e. a jar file and additional metadata) not configuration.
Users that want to programmatically access camel-k Custom Resources with a typed interface using Java can add the dependency to their project and use it as shown in the snippet above.

how the jar will end up into com.github.andreaTP:camel-k:0.0.2-SNAPSHOT

com.github.andreaTP comes from the fact that I published using my fork of this project, if you decide to merge this PR the published artifacts will be named: com.github.apache:camel-k:<version-released>, this is an intrinsic of https://jitpack.io/

I suppose you have some process that will use this configuration for publishing the artifact, is it so?

that's jitpack.io , you can publish the library to Maven Central but this will require more setup/credentials etc.

@squakez
Copy link
Contributor

squakez commented Jan 23, 2023

Thanks for the explaination. So, this is not really going to replace the dependency in a repo, but instead would create one on demand when it's requested by the user. I'm not totally against this approach, but I think it may not be seen as something "official". Also, there may be environment where you're not supposed to use jitpack repo and it requires downloading tools (ie, may not work for other OS). However, I do like the simplicity of the action and I'd propose instead to convert that script into a github action that is run when we create a tag for Camel K repo (which means, we do a release). We may support a simple distribution pom that push the dependency to Apache Maven repo (we do something similar in Camel K Runtime, see https://github.com/apache/camel-k-runtime/blob/main/.github/workflows/ci-build.yml#L196). We can start by creating the action and pushing the snapshot on each PR merge, as an additional step in https://github.com/apache/camel-k/blob/main/.github/actions/release-nightly/action.yml#L103

@andreaTP
Copy link
Contributor Author

Hi @squakez , thanks for bearing with me!

So, this is not really going to replace the dependency in a repo, but instead would create one on demand when it's requested by the user.

this is not accurate, Jitpack does releases (pretty much like Maven Central), so the artifact is going to be built once.

However, I do like the simplicity of the action and I'd propose instead to convert that script into a github action that is run when we create a tag for Camel K repo (which means, we do a release).

This is already the case 😄 , the jitpack configuration will do a full "release" when you do a GH "Release".

push the dependency to Apache Maven repo

Happy to go in this direction if it's the desired one 🙂 , it will be a little difficult for an external contributor to get everything right and I would need some support but we can definitely do it!

@squakez
Copy link
Contributor

squakez commented Jan 23, 2023

This is already the case smile , the jitpack configuration will do a full "release" when you do a GH "Release".

I wasn't aware of this feature and it is definetely interesting.

I still prefer to publish the dependency under the Apache umbrella, wdyt @oscerd?

Happy to go in this direction if it's the desired one slightly_smiling_face , it will be a little difficult for an external contributor to get everything right and I would need some support but we can definitely do it!

Count on it. I think the very first thing is to have a distribution pom with the same configuration of the camel-k-runtime. When we run the push action we'll simply leverage a couple of secrets that we'll configure.

@oscerd
Copy link
Contributor

oscerd commented Jan 23, 2023

I think we should follow the Apache guidlines for releasing this stuff. It's true that this is not really critical and it's more a commodity, but probably we should do the normal workflow.

@andreaTP
Copy link
Contributor Author

Count on it. I think the very first thing is to have a distribution pom with the same configuration of the camel-k-runtime. When we run the push action we'll simply leverage a couple of secrets that we'll configure.

Sounds like a plan 👍 Give me a little to follow up!

@andreaTP
Copy link
Contributor Author

Updated the PR using a pom.xml and a configuration closer to camel-k-runtimes, I do believe that you want to review the artifact name and the version used.

Let me know how this looks to you, and, if possible, please, test it!

@andreaTP andreaTP changed the title [proposal] Publish a generated extension using jbang and jitpack [proposal] Publish a generated extension for Java Jan 23, 2023
Copy link
Contributor

@squakez squakez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor changes required, but the overall work looks good, thanks!

- name: Deploy to ASF Snapshots Repository
working-directory: java
run: |
./mvnw ${MAVEN_ARGS} clean deploy -DskipTests -DskipITs --settings .github/asf-deploy-settings.xml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may use mvn as there is no wrapper available in the java directory. I guess we need an action to install it though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched to mvn, seems like Maven it's already installed:

javaVersion: "11"

java/pom.xml Outdated

<parent>
<groupId>org.apache.camel</groupId>
<artifactId>camel-dependencies</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd avoid depending by Camel project. We're trying to add all decoupling possible. The maven repo distribution managment info are stored in apache pom from which that camel-dependencies inherit. So we better use the apache pom directly:

  <parent>
    <groupId>org.apache</groupId>
    <artifactId>apache</artifactId>
    <version>23</version>
  </parent>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

java/pom.xml Outdated

<modelVersion>4.0.0</modelVersion>
<groupId>org.apache.camel.k</groupId>
<artifactId>camel-k-extension</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer calling it camel-k-model or maybe even camel-k-crd. @oscerd wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched to camel-k-crd, no strong opinions though

<modelVersion>4.0.0</modelVersion>
<groupId>org.apache.camel.k</groupId>
<artifactId>camel-k-extension</artifactId>
<version>1.17.0-SNAPSHOT</version>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to edit the make codegen action to refresh this value, but we can take care of this later on.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏 thanks!

java/.gitignore Outdated Show resolved Hide resolved
@andreaTP
Copy link
Contributor Author

@squakez thanks for the review! Should be ready for the next round!

@squakez
Copy link
Contributor

squakez commented Jan 26, 2023

@oscerd just to double check with you if I can merge this. As a first thing we should verify that snapshot is published correctly through the action process. Once this is okey, we can add the steps required to release the stable version, wdyt?

@oscerd
Copy link
Contributor

oscerd commented Jan 26, 2023

+1 from my side. Thanks

@squakez squakez merged commit 04d4cd7 into apache:main Jan 26, 2023
@squakez squakez added the kind/feature New feature or request label Jan 26, 2023
@andreaTP
Copy link
Contributor Author

🎉 thanks to everyone involved! Feel free to ping me if there is any issue I can help with!

@squakez
Copy link
Contributor

squakez commented Jan 26, 2023

tada thanks to everyone involved! Feel free to ping me if there is any issue I can help with!

yeah, stick around... we may have some other iteration :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants