dhvani7014 commented on PR #6893:
URL: https://github.com/apache/camel-k/pull/6893#issuecomment-6333420688
Thanks a lot for the review @squakez! Here's a proposal before I change the
code:
1. **Pass properties to the scaler.** In `kedaTrait.autoDiscoverTriggers`
(`pkg/trait/keda.go`), collect the Integration's `property` configuration
(`e.collectConfigurationPairs("property")`) plus the camel trait `properties`.
Then change `MapToKedaTrigger(rawURI, props)` and `ScaleMapper.Map(pathValue,
params, props)` in `pkg/trait/keda/mapper.go`, so each scaler only gets the
`camel.component.<scheme>.*` entries for its own scheme. The Kafka scaler keeps
working as it does today.
2. **Artemis mapping, with no hardcoded defaults:**
- `queueName` and `brokerAddress` come from the `amqp:queue:<name>` URI.
- The `managementEndpoint` host comes from
`camel.component.amqp.broker-url`. This works for a plain Integration as well
as a Pipe, since the ArkMQ binding already sets that property.
- If the host can't be resolved, no auto trigger is generated, so users
can still set it through `autoMetadata`.
3. **Open question:** the management port (8161) and `brokerName` aren't in
the Camel config. Should the scaler use 8161 as the default management port and
require `brokerName` through `autoMetadata` for now? Or would you prefer we
read them from the `ActiveMQArtemis` CR?
4. **Credentials** are left out of this first version, as you suggested.
5. **e2e test:** following `e2e/kafka/kafka_autoscale_keda_test.go`, deploy
a plain Integration `from("amqp:queue:my-queue")` with the broker-url property
set, then assert that the ScaledObject has an `artemis-queue` trigger with the
expected metadata.
Does this direction look okay before I implement 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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
