You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "claudio4j (via GitHub)" <gi...@apache.org> on 2024/01/31 17:41:04 UTC
[PR] Fix: add classifier field to the maven artifact abstraction [camel-k-runtime]
claudio4j opened a new pull request, #1161:
URL: https://github.com/apache/camel-k-runtime/pull/1161
Update jolokia to 2.0.0
<!-- Description -->
<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE".
You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes.
-->
**Release Note**
```release-note
feature: add maven artifact classifier to the camel-k-catalog
update: jolokia 2.0.0
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix: add classifier field to the maven artifact abstraction [camel-k-runtime]
Posted by "claudio4j (via GitHub)" <gi...@apache.org>.
claudio4j commented on code in PR #1161:
URL: https://github.com/apache/camel-k-runtime/pull/1161#discussion_r1474356980
##########
support/camel-k-maven-plugin/src/main/java/org/apache/camel/k/tooling/maven/GenerateCatalogMojo.java:
##########
@@ -524,23 +525,26 @@ private void addCapabilities(RuntimeSpec.Builder runtimeSpec, CamelCatalogSpec.B
artifacts.clear();
artifacts.add(Artifact.from("org.apache.camel.quarkus", "camel-quarkus-management"));
artifacts.add(Artifact.from("org.apache.camel.quarkus", "camel-quarkus-jaxb"));
- artifacts.add(Artifact.from("org.jolokia", "jolokia-jvm"));
- addCapabilityAndDependecies(runtimeSpec, catalogSpec, "jolokia", artifacts, false);
+ artifacts.add(Artifact.from("org.jolokia", "jolokia-agent-jvm", "javaagent"));
+ addCapabilityAndDependecies(runtimeSpec, catalogSpec, "jolokia", artifacts, true);
}
private void addCapabilityAndDependecies(RuntimeSpec.Builder runtimeSpec, CamelCatalogSpec.Builder catalogSpec, String name,
- List<Artifact> artifacts, boolean addDependency) {
+ List<Artifact> artifacts, boolean addDependency) {
if (capabilitiesExclusionList != null && !capabilitiesExclusionList.contains(name)) {
CamelCapability.Builder capBuilder = new CamelCapability.Builder();
- artifacts.forEach(artifact -> capBuilder.addDependency(artifact.getGroupId(), artifact.getArtifactId()));
+ artifacts.forEach(artifact -> {
+ capBuilder.addDependency(artifact.getGroupId(), artifact.getArtifactId(), artifact.getClassifier());
+ if (addDependency) {
Review Comment:
The previous code retrieved only the first artifact (see index zero) `artifacts.get(0)`.
With the proposed change, I moved the artifacts collection to the capability definition.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix: add classifier field to the maven artifact abstraction [camel-k-runtime]
Posted by "claudio4j (via GitHub)" <gi...@apache.org>.
claudio4j commented on code in PR #1161:
URL: https://github.com/apache/camel-k-runtime/pull/1161#discussion_r1474394376
##########
support/camel-k-maven-plugin/src/main/java/org/apache/camel/k/tooling/maven/GenerateCatalogMojo.java:
##########
@@ -524,23 +525,26 @@ private void addCapabilities(RuntimeSpec.Builder runtimeSpec, CamelCatalogSpec.B
artifacts.clear();
artifacts.add(Artifact.from("org.apache.camel.quarkus", "camel-quarkus-management"));
artifacts.add(Artifact.from("org.apache.camel.quarkus", "camel-quarkus-jaxb"));
- artifacts.add(Artifact.from("org.jolokia", "jolokia-jvm"));
- addCapabilityAndDependecies(runtimeSpec, catalogSpec, "jolokia", artifacts, false);
+ artifacts.add(Artifact.from("org.jolokia", "jolokia-agent-jvm", "javaagent"));
Review Comment:
jolokia-server-core is not required, I did a test previously, you can try it too:
```
kamel run <sample.java> -t jolokia.enabled=true
kubectl port-forward $(kubectl get po -l camel.apache.org/integration -oname) 8778:8778
camel hawtio
# add the jmx connection to localhost and jolokia path
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix: add classifier field to the maven artifact abstraction [camel-k-runtime]
Posted by "claudio4j (via GitHub)" <gi...@apache.org>.
claudio4j merged PR #1161:
URL: https://github.com/apache/camel-k-runtime/pull/1161
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix: add classifier field to the maven artifact abstraction [camel-k-runtime]
Posted by "claudio4j (via GitHub)" <gi...@apache.org>.
claudio4j commented on code in PR #1161:
URL: https://github.com/apache/camel-k-runtime/pull/1161#discussion_r1474334868
##########
support/camel-k-maven-plugin/src/main/java/org/apache/camel/k/tooling/maven/GenerateCatalogMojo.java:
##########
@@ -524,23 +525,26 @@ private void addCapabilities(RuntimeSpec.Builder runtimeSpec, CamelCatalogSpec.B
artifacts.clear();
artifacts.add(Artifact.from("org.apache.camel.quarkus", "camel-quarkus-management"));
artifacts.add(Artifact.from("org.apache.camel.quarkus", "camel-quarkus-jaxb"));
- artifacts.add(Artifact.from("org.jolokia", "jolokia-jvm"));
- addCapabilityAndDependecies(runtimeSpec, catalogSpec, "jolokia", artifacts, false);
+ artifacts.add(Artifact.from("org.jolokia", "jolokia-agent-jvm", "javaagent"));
Review Comment:
The main artifact is the `jolokia-agent-jvm`, the other dependencies comes transitively from the dependency defined in the camel-k-runtime-bom
The resolved classpath contains the relevant artifacts
```
org.jolokia.jolokia-agent-jvm-2.0.0-javaagent.jar
org.jolokia.jolokia-server-detector-2.0.0.jar
org.jolokia.jolokia-service-discovery-2.0.0.jar
org.jolokia.jolokia-service-history-2.0.0.jar
org.jolokia.jolokia-service-jmx-2.0.0.jar
org.jolokia.jolokia-service-jsr160-2.0.0.jar
org.jolokia.jolokia-service-notif-pull-2.0.0.jar
org.jolokia.jolokia-service-notif-sse-2.0.0.jar
org.jolokia.jolokia-service-serializer-2.0.0.jar
org.apache.camel.camel-management-4.2.0.jar
org.apache.camel.camel-management-api-4.2.0.jar
org.apache.camel.quarkus.camel-quarkus-management-3.6.0.jar
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix: add classifier field to the maven artifact abstraction [camel-k-runtime]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #1161:
URL: https://github.com/apache/camel-k-runtime/pull/1161#discussion_r1473996353
##########
support/camel-k-maven-plugin/src/main/java/org/apache/camel/k/tooling/maven/GenerateCatalogMojo.java:
##########
@@ -524,23 +525,26 @@ private void addCapabilities(RuntimeSpec.Builder runtimeSpec, CamelCatalogSpec.B
artifacts.clear();
artifacts.add(Artifact.from("org.apache.camel.quarkus", "camel-quarkus-management"));
artifacts.add(Artifact.from("org.apache.camel.quarkus", "camel-quarkus-jaxb"));
- artifacts.add(Artifact.from("org.jolokia", "jolokia-jvm"));
- addCapabilityAndDependecies(runtimeSpec, catalogSpec, "jolokia", artifacts, false);
+ artifacts.add(Artifact.from("org.jolokia", "jolokia-agent-jvm", "javaagent"));
Review Comment:
According to the test I've done, we require more dependencies. Here the list: https://github.com/apache/camel-k/pull/5090/commits/9072947913a2d3b5360614711b87d8d7df8818c9#diff-96c9016f9dac002d63c2658bc26f05604540fdb0d51d2c87d978747ce0c41a57R235-R265
##########
support/camel-k-maven-plugin/src/main/java/org/apache/camel/k/tooling/maven/GenerateCatalogMojo.java:
##########
@@ -524,23 +525,26 @@ private void addCapabilities(RuntimeSpec.Builder runtimeSpec, CamelCatalogSpec.B
artifacts.clear();
artifacts.add(Artifact.from("org.apache.camel.quarkus", "camel-quarkus-management"));
artifacts.add(Artifact.from("org.apache.camel.quarkus", "camel-quarkus-jaxb"));
- artifacts.add(Artifact.from("org.jolokia", "jolokia-jvm"));
- addCapabilityAndDependecies(runtimeSpec, catalogSpec, "jolokia", artifacts, false);
+ artifacts.add(Artifact.from("org.jolokia", "jolokia-agent-jvm", "javaagent"));
+ addCapabilityAndDependecies(runtimeSpec, catalogSpec, "jolokia", artifacts, true);
}
private void addCapabilityAndDependecies(RuntimeSpec.Builder runtimeSpec, CamelCatalogSpec.Builder catalogSpec, String name,
- List<Artifact> artifacts, boolean addDependency) {
+ List<Artifact> artifacts, boolean addDependency) {
if (capabilitiesExclusionList != null && !capabilitiesExclusionList.contains(name)) {
CamelCapability.Builder capBuilder = new CamelCapability.Builder();
- artifacts.forEach(artifact -> capBuilder.addDependency(artifact.getGroupId(), artifact.getArtifactId()));
+ artifacts.forEach(artifact -> {
+ capBuilder.addDependency(artifact.getGroupId(), artifact.getArtifactId(), artifact.getClassifier());
+ if (addDependency) {
Review Comment:
Why would this be required?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix: add classifier field to the maven artifact abstraction [camel-k-runtime]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #1161:
URL: https://github.com/apache/camel-k-runtime/pull/1161#discussion_r1474269614
##########
support/camel-k-maven-plugin/src/main/java/org/apache/camel/k/tooling/maven/GenerateCatalogMojo.java:
##########
@@ -524,23 +525,26 @@ private void addCapabilities(RuntimeSpec.Builder runtimeSpec, CamelCatalogSpec.B
artifacts.clear();
artifacts.add(Artifact.from("org.apache.camel.quarkus", "camel-quarkus-management"));
artifacts.add(Artifact.from("org.apache.camel.quarkus", "camel-quarkus-jaxb"));
- artifacts.add(Artifact.from("org.jolokia", "jolokia-jvm"));
- addCapabilityAndDependecies(runtimeSpec, catalogSpec, "jolokia", artifacts, false);
+ artifacts.add(Artifact.from("org.jolokia", "jolokia-agent-jvm", "javaagent"));
+ addCapabilityAndDependecies(runtimeSpec, catalogSpec, "jolokia", artifacts, true);
}
private void addCapabilityAndDependecies(RuntimeSpec.Builder runtimeSpec, CamelCatalogSpec.Builder catalogSpec, String name,
- List<Artifact> artifacts, boolean addDependency) {
+ List<Artifact> artifacts, boolean addDependency) {
if (capabilitiesExclusionList != null && !capabilitiesExclusionList.contains(name)) {
CamelCapability.Builder capBuilder = new CamelCapability.Builder();
- artifacts.forEach(artifact -> capBuilder.addDependency(artifact.getGroupId(), artifact.getArtifactId()));
+ artifacts.forEach(artifact -> {
+ capBuilder.addDependency(artifact.getGroupId(), artifact.getArtifactId(), artifact.getClassifier());
+ if (addDependency) {
Review Comment:
I see. What I do not understand is which is the difference with the previous code which I think was doing the same.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix: add classifier field to the maven artifact abstraction [camel-k-runtime]
Posted by "claudio4j (via GitHub)" <gi...@apache.org>.
claudio4j commented on code in PR #1161:
URL: https://github.com/apache/camel-k-runtime/pull/1161#discussion_r1474246307
##########
support/camel-k-maven-plugin/src/main/java/org/apache/camel/k/tooling/maven/GenerateCatalogMojo.java:
##########
@@ -524,23 +525,26 @@ private void addCapabilities(RuntimeSpec.Builder runtimeSpec, CamelCatalogSpec.B
artifacts.clear();
artifacts.add(Artifact.from("org.apache.camel.quarkus", "camel-quarkus-management"));
artifacts.add(Artifact.from("org.apache.camel.quarkus", "camel-quarkus-jaxb"));
- artifacts.add(Artifact.from("org.jolokia", "jolokia-jvm"));
- addCapabilityAndDependecies(runtimeSpec, catalogSpec, "jolokia", artifacts, false);
+ artifacts.add(Artifact.from("org.jolokia", "jolokia-agent-jvm", "javaagent"));
+ addCapabilityAndDependecies(runtimeSpec, catalogSpec, "jolokia", artifacts, true);
}
private void addCapabilityAndDependecies(RuntimeSpec.Builder runtimeSpec, CamelCatalogSpec.Builder catalogSpec, String name,
- List<Artifact> artifacts, boolean addDependency) {
+ List<Artifact> artifacts, boolean addDependency) {
if (capabilitiesExclusionList != null && !capabilitiesExclusionList.contains(name)) {
CamelCapability.Builder capBuilder = new CamelCapability.Builder();
- artifacts.forEach(artifact -> capBuilder.addDependency(artifact.getGroupId(), artifact.getArtifactId()));
+ artifacts.forEach(artifact -> {
+ capBuilder.addDependency(artifact.getGroupId(), artifact.getArtifactId(), artifact.getClassifier());
+ if (addDependency) {
Review Comment:
This is for the capability to contains all the required dependencies, it's going to generate the following section in the yaml catalog:
```
spec:
runtime:
...
capabilities:
jolokia:
dependencies:
- groupId: org.apache.camel.quarkus
artifactId: camel-quarkus-jaxb
- groupId: org.apache.camel.quarkus
artifactId: camel-quarkus-management
- groupId: org.jolokia
artifactId: jolokia-agent-jvm
classifier: javaagent
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix: add classifier field to the maven artifact abstraction [camel-k-runtime]
Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #1161:
URL: https://github.com/apache/camel-k-runtime/pull/1161#discussion_r1474374012
##########
support/camel-k-maven-plugin/src/main/java/org/apache/camel/k/tooling/maven/GenerateCatalogMojo.java:
##########
@@ -524,23 +525,26 @@ private void addCapabilities(RuntimeSpec.Builder runtimeSpec, CamelCatalogSpec.B
artifacts.clear();
artifacts.add(Artifact.from("org.apache.camel.quarkus", "camel-quarkus-management"));
artifacts.add(Artifact.from("org.apache.camel.quarkus", "camel-quarkus-jaxb"));
- artifacts.add(Artifact.from("org.jolokia", "jolokia-jvm"));
- addCapabilityAndDependecies(runtimeSpec, catalogSpec, "jolokia", artifacts, false);
+ artifacts.add(Artifact.from("org.jolokia", "jolokia-agent-jvm", "javaagent"));
Review Comment:
`jolokia-server-core` seems to be missing. In any case I am not 100% sure of which are the dependencies required as I did some quick try/error experiment. Maybe you can run a test to confirm that nothing else but the agent jar is really required. BTW, since we're there, let's move to 2.0.1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org