Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CAMEL-14385: add a camel-cron component #3474

Merged
merged 8 commits into from
Jan 13, 2020

Conversation

nicolaferraro
Copy link
Member

Copy link
Contributor

@oscerd oscerd left a comment

Choose a reason for hiding this comment

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

At some point we'll need to create the starter in camel-spring-boot repo. Actually we have a dev profile that built against 3.1.0-SNAPSHOT Camel, so you can test the starter already if you want.

rootLogger.level = INFO
rootLogger.appenderRef.file.ref = file

#rootLogger.appenderRef.stdout.ref = stdout
Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove those I believe

@nicolaferraro
Copy link
Member Author

nicolaferraro commented Jan 10, 2020

Thanks @oscerd, I've checked the spring-boot starter with dev profile and it seems to work. I guess it cannot be currently merged, right?

@oscerd
Copy link
Contributor

oscerd commented Jan 10, 2020

Thanks @oscerd, I've checked the spring-boot starter with dev profile and it seems to work. I guess it cannot be currently merged, right?

Yeah, we'll need to create the starters once we release 3.1.0, but it's already a good start :-)


The following standard Camel components support the Cron endpoints:

- Quartz
Copy link
Contributor

Choose a reason for hiding this comment

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

camel-spring also has some cron support via spring as they have cron built-in, so we could also add this there too

/**
* Lazy creation of the CamelCronService
*/
public synchronized void initCamelCronService() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not a fan of this, cant we move this to doStart so its initialized there?

| Name | Description | Default | Type
| *cronService* (advanced) | The id of the CamelCronService to use when multiple implementations are provided | | String
| *basicPropertyBinding* (advanced) | Whether the component should use basic property binding (Camel 2.x) or the newer property binding with additional capabilities | false | boolean
| *lazyStartProducer* (producer) | Whether the producer should be started lazy (on the first message). By starting lazy you can use this to allow CamelContext and routes to startup in situations where a producer may otherwise fail during starting and cause the route to fail being started. By deferring this startup to be lazy then the startup failure can be handled during routing messages via Camel's routing error handlers. Beware that when the first message is processed then creating and starting the producer may take a little time and prolong the total processing time of the processing. | false | boolean
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah okay spotted this, we should fix so this option is not shown for components that are consumer only. Will fix this in camel itself.


<!-- requires camel-core -->
<dependency>
<groupId>org.apache.camel</groupId>
Copy link
Contributor

Choose a reason for hiding this comment

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

What makes it require camel-core, we should ideally only depend on camel-support


@Override
public Consumer createConsumer(Processor processor) throws Exception {
return delegate.createConsumer(processor);
Copy link
Contributor

Choose a reason for hiding this comment

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

You need also to do

configureConsumer(consumer)

}

@Override
public void setSynchronous(boolean synchronous) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yeah we may need to do this on other delegate endpoints (create a jira ticket)

return service;
}

// Fallback to service loader
Copy link
Contributor

Choose a reason for hiding this comment

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

In Camel we have FactoryFinder as our way of loading these kind of stuff. It made it work in OSGi too. However maybe service loader is okay in these modern cloud days.

String uri = uriPath + "?" + query;

QuartzComponent quartz = context.getComponent("quartz", QuartzComponent.class);
return quartz.createEndpoint(uri);
Copy link
Contributor

Choose a reason for hiding this comment

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

Wonder if after we get https://issues.apache.org/jira/browse/CAMEL-14389 done we should start using the type-safe dsl for scenario like this one as it would ease refactoring/deprecations

* The Camel cron component.
*/
@Component("cron")
public class CronComponent extends DefaultComponent {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should implement DelegateEndpoint

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean.. providing kinda DelegateEndpointSupport?

@nicolaferraro
Copy link
Member Author

Thanks for the hints. I shoud have fixed the code.

I've added another implementation in camel-spring, so that it can be used in Spring or Spring-boot oob without pulling quartz.

I've switched to FactoryFinder as well.

@nicolaferraro nicolaferraro merged commit 2a9c609 into apache:master Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants