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 2020/01/09 09:59:43 UTC
[GitHub] [camel-k-runtime] nicolaferraro opened a new pull request #223: Fix
#222: initial implementation of native cron support
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k-runtime] lburgazzoli commented on issue #223: Fix #222:
initial implementation of native cron support
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k-runtime] lburgazzoli commented on issue #223: Fix #222:
initial implementation of native cron support
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k-runtime] lburgazzoli commented on issue #223: Fix #222:
initial implementation of native cron support
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k-runtime] nicolaferraro commented on a change in pull
request #223: Fix #222: initial implementation of native cron support
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k-runtime] lburgazzoli commented on a change in pull request
#223: Fix #222: initial implementation of native cron support
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k-runtime] nicolaferraro merged pull request #223: Fix #222:
initial implementation of native cron support
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k-runtime] lburgazzoli commented on issue #223: Fix #222:
initial implementation of native cron support
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k-runtime] nicolaferraro commented on issue #223: Fix #222:
initial implementation of native cron support
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k-runtime] lburgazzoli edited a comment on issue #223: Fix
#222: initial implementation of native cron support
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k-runtime] nicolaferraro commented on issue #223: Fix #222:
initial implementation of native cron support
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k-runtime] lburgazzoli edited a comment on issue #223: Fix
#222: initial implementation of native cron support
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k-runtime] nicolaferraro commented on issue #223: Fix #222:
initial implementation of native cron support
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k-runtime] lburgazzoli edited a comment on issue #223: Fix
#222: initial implementation of native cron support
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k-runtime] nicolaferraro commented on issue #223: Fix #222:
initial implementation of native cron support
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [camel-k-runtime] nicolaferraro commented on a change in pull
request #223: Fix #222: initial implementation of native cron support
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
With regards,
Apache Git Services