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-kamelet: improve global properties handling #560

Closed
lburgazzoli opened this issue Nov 20, 2020 · 8 comments
Closed

camel-kamelet: improve global properties handling #560

lburgazzoli opened this issue Nov 20, 2020 · 8 comments
Labels
Milestone

Comments

@lburgazzoli
Copy link
Contributor

lburgazzoli commented Nov 20, 2020

The way the kamelet component handles properties as today is:

camel.kamelet.${templateId}
camel.kamelet.${templateId}.${routeId}

But there are some issue with that:

  1. the routeId may be confusing as it can be used for setting a template property and as a root for the materialized route properties
  2. it uses a non standard prefix as all the component use camel.component.${scheme}

A possible alternative is to to leverage camel's capabilities to use map, something like:

camel.component.kamelet.template-properties[${templateId}][foo] = bar
camel.component.kamelet.route-properties[${routeId}][foo] = bar

However it looks quite ugly, a better option would be:

camel.component.kamelet.template-properties[${templateId}].foo = bar
camel.component.kamelet.route-properties[${routeId}].foo = bar

But I don't think camel is currently able to handle this case so we should probably enhance the properties binding support, as example we can mark a component to be PropertiesAware so the binding process should not try to bind the entire object tree but it delegates the mapping to the receiver object.

public interface PropertiesAware {
    void setProperties(Map<String, String> properties);
    Map<String, String> properties getProperties(Map<String, String> properties);
}

Assuming the component would have been something like:

public class KameletComponent extends DefaultComponent {
    private Map<String, PropertiesHolder> templateProperties;
    private Map<String, PropertiesHolder> routeProperties;

    // getter/Setter omitted
}

public class PropertiesHolder implements PropertiesAware {
    @Override
    public void setProperties(Map<String, String> properties) { ... }
    @Override
    public Map<String, String> properties getProperties(Map<String, String> properties) { ... }
}

The binding process then can stop at camel.component.kamelet.template-properties[${templateId}] and camel.component.kamelet.route-properties[${routeId}] and any further processing should be eventually done by the receiver.

@davsclaus does this make any sense ?
@nicolaferraro would the new format be problematic for the operator ? (we'll keep the old way for some time in any case)

@lburgazzoli lburgazzoli added this to the 1.7.0 milestone Nov 20, 2020
@davsclaus
Copy link
Contributor

davsclaus commented Nov 21, 2020

On top of my head there is some components that have property prefix, like camel-http with http client options
https://github.com/apache/camel/blob/master/components/camel-http/src/main/java/org/apache/camel/component/http/HttpComponent.java#L406

(mind that the http component is one of the more complex component for this kind) I think rabbitmq and some others have a bit simpler code for this option prefix options.

Maybe we could have something like that, then its

camel.component.kamelet.template-properties.myTemplate.foo = bar
camel.component.kamelet.template-properties.myTemplate.code = 1234
camel.component.kamelet.route-properties.myRoute.foo = bar

Then template-properties and route-properties are just a Map which we then use that extract code (see link above) and in the kamelet component we can then do the binding we want.

@lburgazzoli
Copy link
Contributor Author

lburgazzoli commented Nov 21, 2020

oh, that's cool and yes that would do what I'm looking for

@lburgazzoli
Copy link
Contributor Author

@davsclaus do you think we can have something for this in 3.7 ?

@davsclaus
Copy link
Contributor

davsclaus commented Nov 23, 2020

Yeah have you tried that extract property thingy? And if so then there should likely only need code changes in camel-kamelet component and not in core camel

@lburgazzoli
Copy link
Contributor Author

not yet

@davsclaus
Copy link
Contributor

Okay if you dont find the time and when I get the compiled simple done, then I can take a look

@lburgazzoli lburgazzoli changed the title camel-kamelet: rimprove global property handling camel-kamelet: improve global properties handling Nov 23, 2020
lburgazzoli added a commit to lburgazzoli/apache-camel-k-runtime that referenced this issue Nov 23, 2020
@lburgazzoli
Copy link
Contributor Author

lburgazzoli commented Nov 24, 2020

Ok, so after we get CAMEL-15886, CAMEL-15887 solved and we get camel 3.7 out then we can get this working. At the moment we can use camel.component.kamelet.configuration.template-properties[myTemplate].foo

lburgazzoli added a commit to lburgazzoli/apache-camel-k-runtime that referenced this issue Nov 24, 2020
@lburgazzoli lburgazzoli modified the milestones: 1.7.0, 1.6.0 Nov 26, 2020
lburgazzoli added a commit to lburgazzoli/apache-camel-k-runtime that referenced this issue Dec 3, 2020
lburgazzoli added a commit to lburgazzoli/apache-camel-k-runtime that referenced this issue Dec 4, 2020
lburgazzoli added a commit to lburgazzoli/apache-camel-k-runtime that referenced this issue Dec 14, 2020
@lburgazzoli
Copy link
Contributor Author

Fixed by #573

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants