[GitHub] [camel-k-runtime] nicolaferraro opened a new pull request #223: Fix #222: initial implementation of native cron support

classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-k-runtime] nicolaferraro opened a new pull request #223: Fix #222: initial implementation of native cron support

GitBox
nicolaferraro opened a new pull request #223: Fix #222: initial implementation of native cron support
URL: https://github.com/apache/camel-k-runtime/pull/223
 
 
   Fix #222
   
   This is a basic impl that needs some help to be improved.
   
   I didn't go the source-loader way because source loaders cannot be easily mixed and this means technically e.g. that you cannot use the cron feature in a knative-source, because both require a distinct source loader (delegation is basic currently).
   
   I've added two customizers, one for `timer` and one for `quartz`. Dependency to the respective camel libraries is optional, so loading a customizer does not take transitive dependencies of the other.
   
   The customizer replaces options at endpoint level. This means that the Endpoint definition (that sometimes is printed in the logs) contains the original URI, but the endpoint values will be different at runtime. I don't know how much this is ok..
   
   There's also a route policy that shuts the route down once the first exchange is completed (gracefully, to give time to all dependent exchanges to complete). This may not be sufficient in the Quarkus case as you mentioned (@davsclaus , @lburgazzoli ), because shutting down the context does not take down the application, right?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-k-runtime] lburgazzoli commented on issue #223: Fix #222: initial implementation of native cron support

GitBox
lburgazzoli commented on issue #223: Fix #222: initial implementation of native cron support
URL: https://github.com/apache/camel-k-runtime/pull/223#issuecomment-572493034
 
 
   Wonder if we should simply replace the input definition of the route with `timer` in any case as we don't need `quartz` overhead.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-k-runtime] nicolaferraro commented on issue #223: Fix #222: initial implementation of native cron support

GitBox
In reply to this post by GitBox
nicolaferraro commented on issue #223: Fix #222: initial implementation of native cron support
URL: https://github.com/apache/camel-k-runtime/pull/223#issuecomment-572507880
 
 
   Good point, but would you remove all quartz libraries on the operator side?
   
   I did minimal variations to make sure an user can switch from kube-scheduled mode to standard mode easily (potentially even without changes in the library path).
   
   I've overridden the required fields, keeping other user settings on the endpoints.. even if I can't find something relevant that an user may customize on quartz. There are some headers added on the exchanges, that can be faked in case.
   
   Both can work..

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-k-runtime] lburgazzoli commented on issue #223: Fix #222: initial implementation of native cron support

GitBox
In reply to this post by GitBox
lburgazzoli commented on issue #223: Fix #222: initial implementation of native cron support
URL: https://github.com/apache/camel-k-runtime/pull/223#issuecomment-572511391
 
 
   > Good point, but would you remove all quartz libraries on the operator side?
   >
   
   Yes, I think in case we want to leverage k8s scheduler we can replace the discovered dependencies with what make more sense for us so in this case quartz --> timer
   
   
   > I did minimal variations to make sure an user can switch from kube-scheduled mode to standard mode easily (potentially even without changes in the library path).
   >
   
   Guess this can be controlled by a trait so the `auto` mode would do the magic but if you need fine grained control, then you can use the trait to influence the level of the magic
   
   > I've overridden the required fields, keeping other user settings on the endpoints.. even if I can't find something relevant that an user may customize on quartz. There are some headers added on the exchanges, that can be faked in case.
   
   I think that if you want k8s jobs, then you do not care about headers generated by the camel components, if you want them, probably you need to keep your pod running as the headers won't be what you expect (i.e. the number of fired event will allays be just 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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-k-runtime] lburgazzoli edited a comment on issue #223: Fix #222: initial implementation of native cron support

GitBox
In reply to this post by GitBox
lburgazzoli edited a comment on issue #223: Fix #222: initial implementation of native cron support
URL: https://github.com/apache/camel-k-runtime/pull/223#issuecomment-572511391
 
 
   > Good point, but would you remove all quartz libraries on the operator side?
   >
   
   Yes, I think in case we want to leverage k8s scheduler we can replace the discovered dependencies with what make more sense for us so in this case quartz --> timer.
   
   BTW, we should also try to determine if you have a single route and in such case we can apply the magic, otherwise we should just go for standard deployment.
   
   > I did minimal variations to make sure an user can switch from kube-scheduled mode to standard mode easily (potentially even without changes in the library path).
   >
   
   Guess this can be controlled by a trait so the `auto` mode would do the magic but if you need fine grained control, then you can use the trait to influence the level of the magic
   
   > I've overridden the required fields, keeping other user settings on the endpoints.. even if I can't find something relevant that an user may customize on quartz. There are some headers added on the exchanges, that can be faked in case.
   
   I think that if you want k8s scheduler, then you do not care about headers generated by the camel components, if you want them, probably you need to keep your pod running as the headers won't be what you expect (i.e. the number of fired event will allays be just 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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-k-runtime] lburgazzoli edited a comment on issue #223: Fix #222: initial implementation of native cron support

GitBox
In reply to this post by GitBox
lburgazzoli edited a comment on issue #223: Fix #222: initial implementation of native cron support
URL: https://github.com/apache/camel-k-runtime/pull/223#issuecomment-572511391
 
 
   > Good point, but would you remove all quartz libraries on the operator side?
   >
   
   Yes, I think in case we want to leverage k8s scheduler we can replace the discovered dependencies with what make more sense for us so in this case quartz --> timer.
   
   BTW, we should also try to determine if you have a single route and in such case we can apply the magic, otherwise we should just go for standard deployment.
   
   > I did minimal variations to make sure an user can switch from kube-scheduled mode to standard mode easily (potentially even without changes in the library path).
   >
   
   Guess this can be controlled by a trait so the `auto` mode would do the magic but if you need fine grained control, then you can use the trait to influence the level of the magic
   
   > I've overridden the required fields, keeping other user settings on the endpoints.. even if I can't find something relevant that an user may customize on quartz. There are some headers added on the exchanges, that can be faked in case.
   
   I think that if you want k8s scheduler, then you do not care about headers generated by the camel components, if you want them, probably you need to keep your pod running as the headers won't be what you expect (i.e. the number of fired event will allways be just 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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-k-runtime] lburgazzoli edited a comment on issue #223: Fix #222: initial implementation of native cron support

GitBox
In reply to this post by GitBox
lburgazzoli edited a comment on issue #223: Fix #222: initial implementation of native cron support
URL: https://github.com/apache/camel-k-runtime/pull/223#issuecomment-572511391
 
 
   > Good point, but would you remove all quartz libraries on the operator side?
   >
   
   Yes, I think in case we want to leverage k8s scheduler we can replace the discovered dependencies with what make more sense for us so in this case quartz --> timer.
   
   BTW, we should also try to determine if you have a single route and in such case we can apply the magic, otherwise we should just go for standard deployment.
   
   > I did minimal variations to make sure an user can switch from kube-scheduled mode to standard mode easily (potentially even without changes in the library path).
   >
   
   Guess this can be controlled by a trait so the `auto` mode would do the magic but if you need fine grained control, then you can use the trait to influence the level of the magic
   
   > I've overridden the required fields, keeping other user settings on the endpoints.. even if I can't find something relevant that an user may customize on quartz. There are some headers added on the exchanges, that can be faked in case.
   
   I think that if you want k8s scheduler, then you do not care about headers generated by the camel components, if you want them, probably you need to keep your pod running as the headers won't be what you expect (i.e. the number of fired event will always be just 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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-k-runtime] nicolaferraro commented on issue #223: Fix #222: initial implementation of native cron support

GitBox
In reply to this post by GitBox
nicolaferraro commented on issue #223: Fix #222: initial implementation of native cron support
URL: https://github.com/apache/camel-k-runtime/pull/223#issuecomment-572514373
 
 
   > > Good point, but would you remove all quartz libraries on the operator side?
   >
   > Yes, I think in case we want to leverage k8s scheduler we can replace the discovered dependencies with what make more sense for us so in this case quartz --> timer
   >
   > > I did minimal variations to make sure an user can switch from kube-scheduled mode to standard mode easily (potentially even without changes in the library path).
   >
   > Guess this can be controlled by a trait so the `auto` mode would do the magic but if you need fine grained control, then you can use the trait to influence the level of the magic
   >
   > > I've overridden the required fields, keeping other user settings on the endpoints.. even if I can't find something relevant that an user may customize on quartz. There are some headers added on the exchanges, that can be faked in case.
   >
   > I think that if you want k8s jobs, then you do not care about headers generated by the camel components, if you want them, probably you need to keep your pod running as the headers won't be what you expect (i.e. the number of fired event will allays be just 1)
   
   Yeah, I was referring to `quartz' that has a fire time... but that can be taken from system time.
   
   Another benefit of the approach is that you can make it generic as it can replace all (ENV var) given endpoints with timers. And that literally allows you to plug new endpoints, directly from the operator.. even inventing new ones.. like `kubernetes-cron:0/1 * * * *`.
   
   Let me do another attempt.. do you think `Runtime.Listener` is a good place to start?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-k-runtime] lburgazzoli commented on issue #223: Fix #222: initial implementation of native cron support

GitBox
In reply to this post by GitBox
lburgazzoli commented on issue #223: Fix #222: initial implementation of native cron support
URL: https://github.com/apache/camel-k-runtime/pull/223#issuecomment-572515581
 
 
   Thinking a little bit more, we can create a `camel-cron` component that like the `camel-platform-http` component can delegate to the a specific implementation

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-k-runtime] nicolaferraro commented on issue #223: Fix #222: initial implementation of native cron support

GitBox
In reply to this post by GitBox
nicolaferraro commented on issue #223: Fix #222: initial implementation of native cron support
URL: https://github.com/apache/camel-k-runtime/pull/223#issuecomment-572518219
 
 
   > Thinking a little bit more, we can create a `camel-cron` component that like the `camel-platform-http` component can delegate to the a specific implementation
   
   That would be nice. A cron trait in the operator can choose the implementation. It will **add** quartz if needed, otherwise falling back on Kube. A reason to use quartz can be also having the route mixed with other active endpoints.
   
   It should be able to support 5 to 7 items in the cron spec.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-k-runtime] lburgazzoli commented on issue #223: Fix #222: initial implementation of native cron support

GitBox
In reply to this post by GitBox
lburgazzoli commented on issue #223: Fix #222: initial implementation of native cron support
URL: https://github.com/apache/camel-k-runtime/pull/223#issuecomment-572518995
 
 
   sounds like a plan :) go ahead

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-k-runtime] nicolaferraro commented on issue #223: Fix #222: initial implementation of native cron support

GitBox
In reply to this post by GitBox
nicolaferraro commented on issue #223: Fix #222: initial implementation of native cron support
URL: https://github.com/apache/camel-k-runtime/pull/223#issuecomment-572593588
 
 
   This second attempt uses a Runtime.Listener to override all specified components, no matter if they are in the classpath.
   
   I'm opening an issue for the Camel `cron` component in the meantime.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-k-runtime] lburgazzoli commented on a change in pull request #223: Fix #222: initial implementation of native cron support

GitBox
In reply to this post by GitBox
lburgazzoli commented on a change in pull request #223: Fix #222: initial implementation of native cron support
URL: https://github.com/apache/camel-k-runtime/pull/223#discussion_r365749872
 
 

 ##########
 File path: camel-k-runtime-cron/src/main/java/org/apache/camel/k/cron/CronRoutePolicyFactory.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.k.cron;
+
+import org.apache.camel.CamelContext;
+import org.apache.camel.Exchange;
+import org.apache.camel.NamedNode;
+import org.apache.camel.Route;
+import org.apache.camel.spi.RoutePolicy;
+import org.apache.camel.spi.RoutePolicyFactory;
+import org.apache.camel.support.RoutePolicySupport;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A RoutePolicyFactory that shuts the context down after the first exchange has been done.
+ */
+public class CronRoutePolicyFactory implements RoutePolicyFactory {
+
+    public CronRoutePolicyFactory() {
+    }
+
+    @Override
+    public RoutePolicy createRoutePolicy(CamelContext camelContext, String routeId, NamedNode route) {
+        return new CronRoutePolicy(camelContext);
+    }
+
+    private static class CronRoutePolicy extends RoutePolicySupport {
+
+        private static final Logger LOG = LoggerFactory.getLogger(CronRoutePolicy.class);
+
+        private final CamelContext context;
+
+        public CronRoutePolicy(CamelContext context) {
+            this.context = context;
+        }
+
+        @Override
+        public void onExchangeDone(Route route, Exchange exchange) {
+            LOG.info("Context shutdown started by cron policy");
+            context.getExecutorServiceManager().newThread("terminator", context::stop).start();
 
 Review comment:
   We probably  need to add a new method `stop()` to the `Runtime` and invoke such method here instead of `CamelContext::stop` so the standard runtime would invoke `Main::stop` and the Quarkus one will invoke the correct method once provided by quarkus

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-k-runtime] nicolaferraro commented on a change in pull request #223: Fix #222: initial implementation of native cron support

GitBox
In reply to this post by GitBox
nicolaferraro commented on a change in pull request #223: Fix #222: initial implementation of native cron support
URL: https://github.com/apache/camel-k-runtime/pull/223#discussion_r365757076
 
 

 ##########
 File path: camel-k-runtime-cron/src/main/java/org/apache/camel/k/cron/CronRoutePolicyFactory.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.k.cron;
+
+import org.apache.camel.CamelContext;
+import org.apache.camel.Exchange;
+import org.apache.camel.NamedNode;
+import org.apache.camel.Route;
+import org.apache.camel.spi.RoutePolicy;
+import org.apache.camel.spi.RoutePolicyFactory;
+import org.apache.camel.support.RoutePolicySupport;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A RoutePolicyFactory that shuts the context down after the first exchange has been done.
+ */
+public class CronRoutePolicyFactory implements RoutePolicyFactory {
+
+    public CronRoutePolicyFactory() {
+    }
+
+    @Override
+    public RoutePolicy createRoutePolicy(CamelContext camelContext, String routeId, NamedNode route) {
+        return new CronRoutePolicy(camelContext);
+    }
+
+    private static class CronRoutePolicy extends RoutePolicySupport {
+
+        private static final Logger LOG = LoggerFactory.getLogger(CronRoutePolicy.class);
+
+        private final CamelContext context;
+
+        public CronRoutePolicy(CamelContext context) {
+            this.context = context;
+        }
+
+        @Override
+        public void onExchangeDone(Route route, Exchange exchange) {
+            LOG.info("Context shutdown started by cron policy");
+            context.getExecutorServiceManager().newThread("terminator", context::stop).start();
 
 Review comment:
   Yeah, sounds good!

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-k-runtime] nicolaferraro commented on a change in pull request #223: Fix #222: initial implementation of native cron support

GitBox
In reply to this post by GitBox
nicolaferraro commented on a change in pull request #223: Fix #222: initial implementation of native cron support
URL: https://github.com/apache/camel-k-runtime/pull/223#discussion_r365863121
 
 

 ##########
 File path: camel-k-runtime-cron/src/main/java/org/apache/camel/k/cron/CronRoutePolicyFactory.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.k.cron;
+
+import org.apache.camel.CamelContext;
+import org.apache.camel.Exchange;
+import org.apache.camel.NamedNode;
+import org.apache.camel.Route;
+import org.apache.camel.spi.RoutePolicy;
+import org.apache.camel.spi.RoutePolicyFactory;
+import org.apache.camel.support.RoutePolicySupport;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A RoutePolicyFactory that shuts the context down after the first exchange has been done.
+ */
+public class CronRoutePolicyFactory implements RoutePolicyFactory {
+
+    public CronRoutePolicyFactory() {
+    }
+
+    @Override
+    public RoutePolicy createRoutePolicy(CamelContext camelContext, String routeId, NamedNode route) {
+        return new CronRoutePolicy(camelContext);
+    }
+
+    private static class CronRoutePolicy extends RoutePolicySupport {
+
+        private static final Logger LOG = LoggerFactory.getLogger(CronRoutePolicy.class);
+
+        private final CamelContext context;
+
+        public CronRoutePolicy(CamelContext context) {
+            this.context = context;
+        }
+
+        @Override
+        public void onExchangeDone(Route route, Exchange exchange) {
+            LOG.info("Context shutdown started by cron policy");
+            context.getExecutorServiceManager().newThread("terminator", context::stop).start();
 
 Review comment:
   I've updated it

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-k-runtime] nicolaferraro merged pull request #223: Fix #222: initial implementation of native cron support

GitBox
In reply to this post by GitBox
nicolaferraro merged pull request #223: Fix #222: initial implementation of native cron support
URL: https://github.com/apache/camel-k-runtime/pull/223
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services