You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2022/09/13 23:02:49 UTC
[GitHub] [camel-quarkus] javaduke opened a new pull request, #4088: JCache native mode
javaduke opened a new pull request, #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088
Addresses issue #1846
--
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
[GitHub] [camel-quarkus] aldettinger commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
aldettinger commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970665950
##########
integration-tests/jcache/src/test/java/org/apache/camel/quarkus/component/jcache/it/JcacheTest.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.jcache.it;
+
+import io.quarkus.test.junit.QuarkusTest;
+import io.restassured.RestAssured;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.Matchers.is;
+
+@QuarkusTest
+class JcacheTest {
+
+ @Test
+ public void loadComponentJcache() {
+ /* A simple autogenerated test */
+ RestAssured.get("/jcache/load/component/jcache")
+ .then()
+ .statusCode(200);
+ }
+
+ @Test
+ public void testCache() {
+ RestAssured.get("/jcache/sayHello")
+ .then()
+ .statusCode(200)
+ .body(is("Hello World : Non-Cached Response"));
+
+ RestAssured.get("/jcache/sayHello")
+ .then()
+ .statusCode(200)
+ .body(is("Hello World : Cached Response"));
+
+ //Wait till Cache expires
+ try {
+ Thread.sleep(12000);
Review Comment:
How do we know it is 12s ? It's a like a cache default value of 10s + 2s security ?
--
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
[GitHub] [camel-quarkus] javaduke commented on pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
javaduke commented on PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#issuecomment-1247053635
Hmm, not sure why this one check is failing, not my fault :)
--
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
[GitHub] [camel-quarkus] javaduke commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
javaduke commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970833293
##########
integration-tests/jcache/src/main/java/org/apache/camel/quarkus/component/jcache/it/JcacheCaffeineReflectionConfiguration.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.jcache.it;
+
+import io.quarkus.runtime.annotations.RegisterForReflection;
+
+@RegisterForReflection(classNames = {
Review Comment:
Actually I think the new `caffeine-jcache` extension should be part of the Quarkus itself, not `camel-quarkus`, because caffeine jcache has really nothing to do with Camel. I'll create another PR to Quarkus and if it is accepted, I'll remove this class altogether.
--
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
[GitHub] [camel-quarkus] javaduke commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
javaduke commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970843896
##########
integration-tests/jcache/src/test/java/org/apache/camel/quarkus/component/jcache/it/JcacheTest.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.jcache.it;
+
+import io.quarkus.test.junit.QuarkusTest;
+import io.restassured.RestAssured;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.Matchers.is;
+
+@QuarkusTest
+class JcacheTest {
+
+ @Test
+ public void loadComponentJcache() {
+ /* A simple autogenerated test */
+ RestAssured.get("/jcache/load/component/jcache")
+ .then()
+ .statusCode(200);
+ }
+
+ @Test
+ public void testCache() {
+ RestAssured.get("/jcache/sayHello")
+ .then()
+ .statusCode(200)
+ .body(is("Hello World : Non-Cached Response"));
+
+ RestAssured.get("/jcache/sayHello")
+ .then()
+ .statusCode(200)
+ .body(is("Hello World : Cached Response"));
+
+ //Wait till Cache expires
+ try {
+ Thread.sleep(12000);
Review Comment:
OK, I changed it to 3 seconds cache expiration and 4 seconds sleep, is it ok?
--
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
[GitHub] [camel-quarkus] javaduke commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
javaduke commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970856574
##########
integration-tests/jcache/src/main/java/org/apache/camel/quarkus/component/jcache/it/JcacheCaffeineReflectionConfiguration.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.jcache.it;
+
+import io.quarkus.runtime.annotations.RegisterForReflection;
+
+@RegisterForReflection(classNames = {
Review Comment:
Sure, what I meant is if and after the Quarkus supports `caffeine-jcache`, we can revisit this one and remove this class. But for now I guess it's not big harm to have it here.
--
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
[GitHub] [camel-quarkus] ppalaga commented on pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
ppalaga commented on PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#issuecomment-1246628882
Thanks @javaduke. This is good to merge after addressing the small nitpicks found by @zhfeng @aldettinger and me. Plus we hope to get a followup PR for https://github.com/apache/camel-quarkus/issues/4092 soon.
--
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
[GitHub] [camel-quarkus] javaduke commented on pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
javaduke commented on PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#issuecomment-1247169470
Strange, now it's FTP test that is failing...
--
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
[GitHub] [camel-quarkus] aldettinger commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
aldettinger commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970662228
##########
integration-tests/jcache/src/main/java/org/apache/camel/quarkus/component/jcache/it/JcacheResource.java:
##########
@@ -37,6 +37,9 @@ public class JcacheResource {
@Inject
CamelContext context;
+ @Inject
+ ProducerTemplate producerTemplate;
+
Review Comment:
Note that method like `loadComponentJcache` are generally removed as they don't provide that much value anymore in native mode.
--
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
[GitHub] [camel-quarkus] aldettinger commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
aldettinger commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970664214
##########
integration-tests/jcache/src/test/java/org/apache/camel/quarkus/component/jcache/it/JcacheTest.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.jcache.it;
+
+import io.quarkus.test.junit.QuarkusTest;
+import io.restassured.RestAssured;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.Matchers.is;
+
+@QuarkusTest
+class JcacheTest {
+
+ @Test
+ public void loadComponentJcache() {
Review Comment:
Same way, we generally remove such jvm only test.
--
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
[GitHub] [camel-quarkus] ppalaga commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
ppalaga commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970943697
##########
integration-tests/jcache/src/test/java/org/apache/camel/quarkus/component/jcache/it/JcacheTest.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.jcache.it;
+
+import io.quarkus.test.junit.QuarkusTest;
+import io.restassured.RestAssured;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.Matchers.is;
+
+@QuarkusTest
+class JcacheTest {
+
+ @Test
+ public void loadComponentJcache() {
+ /* A simple autogenerated test */
+ RestAssured.get("/jcache/load/component/jcache")
+ .then()
+ .statusCode(200);
+ }
+
+ @Test
+ public void testCache() {
+ RestAssured.get("/jcache/sayHello")
+ .then()
+ .statusCode(200)
+ .body(is("Hello World : Non-Cached Response"));
+
+ RestAssured.get("/jcache/sayHello")
+ .then()
+ .statusCode(200)
+ .body(is("Hello World : Cached Response"));
+
+ //Wait till Cache expires
+ try {
+ Thread.sleep(12000);
Review Comment:
> OK, I changed it to 3 seconds cache expiration and 4 seconds sleep, is it ok?
Much better, thanks!
--
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
[GitHub] [camel-quarkus] javaduke commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
javaduke commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970266507
##########
extensions/jcache/deployment/src/main/java/org/apache/camel/quarkus/component/jcache/deployment/JcacheProcessor.java:
##########
@@ -34,13 +38,20 @@ FeatureBuildItem feature() {
return new FeatureBuildItem(FEATURE);
}
- /**
- * Remove this once this extension starts supporting the native mode.
- */
- @BuildStep(onlyIf = NativeBuild.class)
- @Record(value = ExecutionTime.RUNTIME_INIT)
- void warnJvmInNative(JvmOnlyRecorder recorder) {
- JvmOnlyRecorder.warnJvmInNative(LOG, FEATURE); // warn at build time
- recorder.warnJvmInNative(FEATURE); // warn at runtime
+ @BuildStep
+ void nativeResources(BuildProducer<ServiceProviderBuildItem> services,
+ BuildProducer<NativeImageResourceBuildItem> nativeResources) {
Review Comment:
@ppalaga suggested this, see here: https://github.com/apache/camel-quarkus/issues/1846#issuecomment-1245107767 - the purpose is to discover caching providers.
--
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
[GitHub] [camel-quarkus] javaduke commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
javaduke commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970266843
##########
integration-tests/jcache/src/main/resources/application.properties:
##########
@@ -0,0 +1,17 @@
+## ---------------------------------------------------------------------------
+## Licensed to the Apache Software Foundation (ASF) under one or more
+## contributor license agreements. See the NOTICE file distributed with
+## this work for additional information regarding copyright ownership.
+## The ASF licenses this file to You under the Apache License, Version 2.0
+## (the "License"); you may not use this file except in compliance with
+## the License. You may obtain a copy of the License at
+##
+## http://www.apache.org/licenses/LICENSE-2.0
+##
+## Unless required by applicable law or agreed to in writing, software
+## distributed under the License is distributed on an "AS IS" BASIS,
+## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+## See the License for the specific language governing permissions and
+## limitations under the License.
+## ---------------------------------------------------------------------------
+quarkus.native.resources.includes=reference.conf
Review Comment:
This is located in caffeine jcache jar, it is a default configuration file.
--
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
[GitHub] [camel-quarkus] aldettinger commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
aldettinger commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970853719
##########
integration-tests/jcache/src/main/java/org/apache/camel/quarkus/component/jcache/it/JcacheCaffeineReflectionConfiguration.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.jcache.it;
+
+import io.quarkus.runtime.annotations.RegisterForReflection;
+
+@RegisterForReflection(classNames = {
Review Comment:
There is value in merging this PR now and refactoring afterward. As you want.
--
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
[GitHub] [camel-quarkus] aldettinger merged pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
aldettinger merged PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088
--
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
[GitHub] [camel-quarkus] ppalaga commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
ppalaga commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970667641
##########
extensions/jcache/deployment/src/main/java/org/apache/camel/quarkus/component/jcache/deployment/JcacheProcessor.java:
##########
@@ -34,13 +38,20 @@ FeatureBuildItem feature() {
return new FeatureBuildItem(FEATURE);
}
- /**
- * Remove this once this extension starts supporting the native mode.
- */
- @BuildStep(onlyIf = NativeBuild.class)
- @Record(value = ExecutionTime.RUNTIME_INIT)
- void warnJvmInNative(JvmOnlyRecorder recorder) {
- JvmOnlyRecorder.warnJvmInNative(LOG, FEATURE); // warn at build time
- recorder.warnJvmInNative(FEATURE); // warn at runtime
+ @BuildStep
+ void nativeResources(BuildProducer<ServiceProviderBuildItem> services,
+ BuildProducer<NativeImageResourceBuildItem> nativeResources) {
Review Comment:
Yes, `BuildProducer<NativeImageResourceBuildItem> nativeResources` are not necessary. Sorry for the confusion.
--
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
[GitHub] [camel-quarkus] aldettinger commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
aldettinger commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970660322
##########
extensions/jcache/deployment/src/main/java/org/apache/camel/quarkus/component/jcache/deployment/JcacheProcessor.java:
##########
@@ -34,13 +38,20 @@ FeatureBuildItem feature() {
return new FeatureBuildItem(FEATURE);
}
- /**
- * Remove this once this extension starts supporting the native mode.
- */
- @BuildStep(onlyIf = NativeBuild.class)
- @Record(value = ExecutionTime.RUNTIME_INIT)
- void warnJvmInNative(JvmOnlyRecorder recorder) {
- JvmOnlyRecorder.warnJvmInNative(LOG, FEATURE); // warn at build time
- recorder.warnJvmInNative(FEATURE); // warn at runtime
+ @BuildStep
+ void nativeResources(BuildProducer<ServiceProviderBuildItem> services,
+ BuildProducer<NativeImageResourceBuildItem> nativeResources) {
Review Comment:
Indeed, the `nativeResources` method param looks not to be used. Also, I wonder whether methods like `ServiceProviderBuildItem.allProvider*` could help here.
--
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
[GitHub] [camel-quarkus] javaduke commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
javaduke commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970945112
##########
integration-tests/jcache/src/main/java/org/apache/camel/quarkus/component/jcache/it/JcacheCaffeineReflectionConfiguration.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.jcache.it;
+
+import io.quarkus.runtime.annotations.RegisterForReflection;
+
+@RegisterForReflection(classNames = {
Review Comment:
Hmm, yeah, I thought about this, but maybe the easiest would be just to add jcache support to the existing quarkus caffeine extension.
As for the service provider registration, no, I think it has to stay here, again, because there can be any provider, not just caffeine, it has to be generic, so should be discoverable by this extension.
--
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
[GitHub] [camel-quarkus] ppalaga commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
ppalaga commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970680372
##########
integration-tests/jcache/src/main/java/org/apache/camel/quarkus/component/jcache/it/JcacheCaffeineReflectionConfiguration.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.jcache.it;
+
+import io.quarkus.runtime.annotations.RegisterForReflection;
+
+@RegisterForReflection(classNames = {
Review Comment:
```suggestion
/** These reflection registrations should be moved to a new camel-quarkus-jcache-caffeine extension - see https://github.com/apache/camel-quarkus/issues/4092 */
@RegisterForReflection(classNames = {
```
--
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
[GitHub] [camel-quarkus] javaduke commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
javaduke commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970842915
##########
extensions/jcache/deployment/src/main/java/org/apache/camel/quarkus/component/jcache/deployment/JcacheProcessor.java:
##########
@@ -34,13 +38,20 @@ FeatureBuildItem feature() {
return new FeatureBuildItem(FEATURE);
}
- /**
- * Remove this once this extension starts supporting the native mode.
- */
- @BuildStep(onlyIf = NativeBuild.class)
- @Record(value = ExecutionTime.RUNTIME_INIT)
- void warnJvmInNative(JvmOnlyRecorder recorder) {
- JvmOnlyRecorder.warnJvmInNative(LOG, FEATURE); // warn at build time
- recorder.warnJvmInNative(FEATURE); // warn at runtime
+ @BuildStep
+ void nativeResources(BuildProducer<ServiceProviderBuildItem> services,
+ BuildProducer<NativeImageResourceBuildItem> nativeResources) {
Review Comment:
Done, removed
##########
integration-tests/jcache/src/main/java/org/apache/camel/quarkus/component/jcache/it/JcacheResource.java:
##########
@@ -37,6 +37,9 @@ public class JcacheResource {
@Inject
CamelContext context;
+ @Inject
+ ProducerTemplate producerTemplate;
+
Review Comment:
Done
--
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
[GitHub] [camel-quarkus] ppalaga commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
ppalaga commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970941687
##########
integration-tests/jcache/src/main/java/org/apache/camel/quarkus/component/jcache/it/JcacheCaffeineReflectionConfiguration.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.jcache.it;
+
+import io.quarkus.runtime.annotations.RegisterForReflection;
+
+@RegisterForReflection(classNames = {
Review Comment:
> Actually I think the new `caffeine-jcache` extension should be part of the Quarkus itself, not `camel-quarkus`, because caffeine jcache has really nothing to do with Camel. I'll create another PR to Quarkus and if it is accepted, I'll remove this class altogether.
Good luck with it! I bet they'll recommend creating a new Quarkiverse repo for that, which I'd personally not prefer because of the maintenance overhead. Note that you'd also have to move the service provider registration to a new jcache extension.
--
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
[GitHub] [camel-quarkus] aldettinger commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
aldettinger commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970849810
##########
integration-tests/jcache/src/test/java/org/apache/camel/quarkus/component/jcache/it/JcacheTest.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.jcache.it;
+
+import io.quarkus.test.junit.QuarkusTest;
+import io.restassured.RestAssured;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.Matchers.is;
+
+@QuarkusTest
+class JcacheTest {
+
+ @Test
+ public void loadComponentJcache() {
+ /* A simple autogenerated test */
+ RestAssured.get("/jcache/load/component/jcache")
+ .then()
+ .statusCode(200);
+ }
+
+ @Test
+ public void testCache() {
+ RestAssured.get("/jcache/sayHello")
+ .then()
+ .statusCode(200)
+ .body(is("Hello World : Non-Cached Response"));
+
+ RestAssured.get("/jcache/sayHello")
+ .then()
+ .statusCode(200)
+ .body(is("Hello World : Cached Response"));
+
+ //Wait till Cache expires
+ try {
+ Thread.sleep(12000);
Review Comment:
LGTM
--
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
[GitHub] [camel-quarkus] aldettinger commented on pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
aldettinger commented on PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#issuecomment-1247072897
Highly likely a download issue. Let's re-run the failed job to be sure.
--
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
[GitHub] [camel-quarkus] javaduke commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
javaduke commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970843293
##########
integration-tests/jcache/src/test/java/org/apache/camel/quarkus/component/jcache/it/JcacheTest.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.jcache.it;
+
+import io.quarkus.test.junit.QuarkusTest;
+import io.restassured.RestAssured;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.Matchers.is;
+
+@QuarkusTest
+class JcacheTest {
+
+ @Test
+ public void loadComponentJcache() {
Review Comment:
Done
--
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
[GitHub] [camel-quarkus] aldettinger commented on pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
aldettinger commented on PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#issuecomment-1247249032
This one is a long time [flaky test](https://github.com/apache/camel-quarkus/issues/2810), sure it's not linked to the PR.
Let's merge then.
--
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
[GitHub] [camel-quarkus] ppalaga commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
ppalaga commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970669975
##########
integration-tests/jcache/src/test/java/org/apache/camel/quarkus/component/jcache/it/JcacheTest.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.quarkus.component.jcache.it;
+
+import io.quarkus.test.junit.QuarkusTest;
+import io.restassured.RestAssured;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.Matchers.is;
+
+@QuarkusTest
+class JcacheTest {
+
+ @Test
+ public void loadComponentJcache() {
+ /* A simple autogenerated test */
+ RestAssured.get("/jcache/load/component/jcache")
+ .then()
+ .statusCode(200);
+ }
+
+ @Test
+ public void testCache() {
+ RestAssured.get("/jcache/sayHello")
+ .then()
+ .statusCode(200)
+ .body(is("Hello World : Non-Cached Response"));
+
+ RestAssured.get("/jcache/sayHello")
+ .then()
+ .statusCode(200)
+ .body(is("Hello World : Cached Response"));
+
+ //Wait till Cache expires
+ try {
+ Thread.sleep(12000);
Review Comment:
I'd vote for a shorter cache expiration, if possible. 12s sounds too much.
--
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
[GitHub] [camel-quarkus] zhfeng commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
zhfeng commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970265256
##########
extensions/jcache/deployment/src/main/java/org/apache/camel/quarkus/component/jcache/deployment/JcacheProcessor.java:
##########
@@ -34,13 +38,20 @@ FeatureBuildItem feature() {
return new FeatureBuildItem(FEATURE);
}
- /**
- * Remove this once this extension starts supporting the native mode.
- */
- @BuildStep(onlyIf = NativeBuild.class)
- @Record(value = ExecutionTime.RUNTIME_INIT)
- void warnJvmInNative(JvmOnlyRecorder recorder) {
- JvmOnlyRecorder.warnJvmInNative(LOG, FEATURE); // warn at build time
- recorder.warnJvmInNative(FEATURE); // warn at runtime
+ @BuildStep
+ void nativeResources(BuildProducer<ServiceProviderBuildItem> services,
+ BuildProducer<NativeImageResourceBuildItem> nativeResources) {
Review Comment:
It seems that `nativeResources` does not be used in the method. Did you want to regiter some resouces in native image?
##########
integration-tests/jcache/src/main/resources/application.properties:
##########
@@ -0,0 +1,17 @@
+## ---------------------------------------------------------------------------
+## Licensed to the Apache Software Foundation (ASF) under one or more
+## contributor license agreements. See the NOTICE file distributed with
+## this work for additional information regarding copyright ownership.
+## The ASF licenses this file to You under the Apache License, Version 2.0
+## (the "License"); you may not use this file except in compliance with
+## the License. You may obtain a copy of the License at
+##
+## http://www.apache.org/licenses/LICENSE-2.0
+##
+## Unless required by applicable law or agreed to in writing, software
+## distributed under the License is distributed on an "AS IS" BASIS,
+## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+## See the License for the specific language governing permissions and
+## limitations under the License.
+## ---------------------------------------------------------------------------
+quarkus.native.resources.includes=reference.conf
Review Comment:
I can not find `reference.conf` in the test resource. Why we need this file?
--
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
[GitHub] [camel-quarkus] zhfeng commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
zhfeng commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970278736
##########
integration-tests/jcache/src/main/resources/application.properties:
##########
@@ -0,0 +1,17 @@
+## ---------------------------------------------------------------------------
+## Licensed to the Apache Software Foundation (ASF) under one or more
+## contributor license agreements. See the NOTICE file distributed with
+## this work for additional information regarding copyright ownership.
+## The ASF licenses this file to You under the Apache License, Version 2.0
+## (the "License"); you may not use this file except in compliance with
+## the License. You may obtain a copy of the License at
+##
+## http://www.apache.org/licenses/LICENSE-2.0
+##
+## Unless required by applicable law or agreed to in writing, software
+## distributed under the License is distributed on an "AS IS" BASIS,
+## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+## See the License for the specific language governing permissions and
+## limitations under the License.
+## ---------------------------------------------------------------------------
+quarkus.native.resources.includes=reference.conf
Review Comment:
Thanks for explanation.
--
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
[GitHub] [camel-quarkus] zhfeng commented on a diff in pull request #4088: JCache native mode
Posted by GitBox <gi...@apache.org>.
zhfeng commented on code in PR #4088:
URL: https://github.com/apache/camel-quarkus/pull/4088#discussion_r970277955
##########
extensions/jcache/deployment/src/main/java/org/apache/camel/quarkus/component/jcache/deployment/JcacheProcessor.java:
##########
@@ -34,13 +38,20 @@ FeatureBuildItem feature() {
return new FeatureBuildItem(FEATURE);
}
- /**
- * Remove this once this extension starts supporting the native mode.
- */
- @BuildStep(onlyIf = NativeBuild.class)
- @Record(value = ExecutionTime.RUNTIME_INIT)
- void warnJvmInNative(JvmOnlyRecorder recorder) {
- JvmOnlyRecorder.warnJvmInNative(LOG, FEATURE); // warn at build time
- recorder.warnJvmInNative(FEATURE); // warn at runtime
+ @BuildStep
+ void nativeResources(BuildProducer<ServiceProviderBuildItem> services,
+ BuildProducer<NativeImageResourceBuildItem> nativeResources) {
Review Comment:
OK, I get it. So it looks like only need `BuildProducer<ServiceProviderBuildItem> services` here. Is it right? @ppalaga
--
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