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/23 11:43:04 UTC
[GitHub] [camel-quarkus] lburgazzoli opened a new pull request #660: The
MicroProfile test fails if message history is turned off
lburgazzoli opened a new pull request #660: The MicroProfile test fails if message history is turned off
URL: https://github.com/apache/camel-quarkus/pull/660
Fixes #650
----------------------------------------------------------------
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-quarkus] lburgazzoli edited a comment on issue #660: The
MicroProfile test fails if message history is turned off
Posted by GitBox <gi...@apache.org>.
lburgazzoli edited a comment on issue #660: The MicroProfile test fails if message history is turned off
URL: https://github.com/apache/camel-quarkus/pull/660#issuecomment-577646452
@ppalaga @jamesnetherton do you mind having a look ?
need to be better tested but haven't yet found a easy way to check agains different set of properties without creating ad additional test module (we can create a new one that tests only in jvm mode)
----------------------------------------------------------------
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-quarkus] lburgazzoli commented on a change in pull request
#660: The MicroProfile test fails if message history is turned off
Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on a change in pull request #660: The MicroProfile test fails if message history is turned off
URL: https://github.com/apache/camel-quarkus/pull/660#discussion_r370087866
##########
File path: extensions/microprofile-metrics/runtime/src/main/java/org/apache/camel/quarkus/component/microprofile/metrics/runtime/CamelMicroProfileMetricsRecorder.java
##########
@@ -61,4 +63,24 @@ public void configureCamelContext(CamelMicroProfileMetricsConfig config,
managementStrategy.addEventNotifier(new MicroProfileMetricsCamelContextEventNotifier());
}
}
+
+ private static class MicroProfileMetricsContextConfigurerListener extends MainListenerSupport {
+ private static final Logger LOGGER = Logger.getLogger(MicroProfileMetricsContextConfigurerListener.class);
+ private final CamelMicroProfileMetricsConfig config;
+
+ public MicroProfileMetricsContextConfigurerListener(CamelMicroProfileMetricsConfig config) {
+ this.config = config;
+ }
+
+ @Override
+ public void configure(CamelContext camelContext) {
+ if (camelContext.isMessageHistory() && config.enableMessageHistory) {
Review comment:
It can certainly be done, my only reasoning was: what if the user has explicitly disabled message history ? by adding the extension we would then revert its change without notice.
I guess I can add a log that say that the message history has been turned on in case it was not, maybe the same should happen on 3.1.x.
----------------------------------------------------------------
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-quarkus] lburgazzoli commented on issue #660: The
MicroProfile test fails if message history is turned off
Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on issue #660: The MicroProfile test fails if message history is turned off
URL: https://github.com/apache/camel-quarkus/pull/660#issuecomment-577646452
@ppalaga @jamesnetherton do you mind having a look ?
need to be better tested but haven't yet found a easy way to check agains different set of properties without creating ad additional test module
----------------------------------------------------------------
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-quarkus] lburgazzoli commented on a change in pull request
#660: The MicroProfile test fails if message history is turned off
Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on a change in pull request #660: The MicroProfile test fails if message history is turned off
URL: https://github.com/apache/camel-quarkus/pull/660#discussion_r370094527
##########
File path: extensions/microprofile-metrics/runtime/src/main/java/org/apache/camel/quarkus/component/microprofile/metrics/runtime/CamelMicroProfileMetricsRecorder.java
##########
@@ -61,4 +63,24 @@ public void configureCamelContext(CamelMicroProfileMetricsConfig config,
managementStrategy.addEventNotifier(new MicroProfileMetricsCamelContextEventNotifier());
}
}
+
+ private static class MicroProfileMetricsContextConfigurerListener extends MainListenerSupport {
+ private static final Logger LOGGER = Logger.getLogger(MicroProfileMetricsContextConfigurerListener.class);
+ private final CamelMicroProfileMetricsConfig config;
+
+ public MicroProfileMetricsContextConfigurerListener(CamelMicroProfileMetricsConfig config) {
+ this.config = config;
+ }
+
+ @Override
+ public void configure(CamelContext camelContext) {
+ if (camelContext.isMessageHistory() && config.enableMessageHistory) {
Review comment:
@jamesnetherton I've simplified it a bit, does it look better now ?
----------------------------------------------------------------
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-quarkus] lburgazzoli merged pull request #660: The
MicroProfile test fails if message history is turned off
Posted by GitBox <gi...@apache.org>.
lburgazzoli merged pull request #660: The MicroProfile test fails if message history is turned off
URL: https://github.com/apache/camel-quarkus/pull/660
----------------------------------------------------------------
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-quarkus] jamesnetherton commented on a change in pull
request #660: The MicroProfile test fails if message history is turned off
Posted by GitBox <gi...@apache.org>.
jamesnetherton commented on a change in pull request #660: The MicroProfile test fails if message history is turned off
URL: https://github.com/apache/camel-quarkus/pull/660#discussion_r370078501
##########
File path: extensions/microprofile-metrics/runtime/src/main/java/org/apache/camel/quarkus/component/microprofile/metrics/runtime/CamelMicroProfileMetricsRecorder.java
##########
@@ -61,4 +63,24 @@ public void configureCamelContext(CamelMicroProfileMetricsConfig config,
managementStrategy.addEventNotifier(new MicroProfileMetricsCamelContextEventNotifier());
}
}
+
+ private static class MicroProfileMetricsContextConfigurerListener extends MainListenerSupport {
+ private static final Logger LOGGER = Logger.getLogger(MicroProfileMetricsContextConfigurerListener.class);
+ private final CamelMicroProfileMetricsConfig config;
+
+ public MicroProfileMetricsContextConfigurerListener(CamelMicroProfileMetricsConfig config) {
+ this.config = config;
+ }
+
+ @Override
+ public void configure(CamelContext camelContext) {
+ if (camelContext.isMessageHistory() && config.enableMessageHistory) {
Review comment:
I wonder if we can simplify this to just:
```
camelContext.setMessageHistory(true);
camelContext.setMessageHistoryFactory(new MicroProfileMetricsMessageHistoryFactory());
```
And not bother with the other checks?
In 3.1.0, when you add the `MessageHistoryFactory`, message history is [auto enabled](https://github.com/apache/camel/blob/f984a85ced885e0f8da58d3477b703d9cc2ff88c/core/camel-base/src/main/java/org/apache/camel/impl/engine/AbstractCamelContext.java#L3887) on the `CamelContext` anyway.
----------------------------------------------------------------
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