Skip to content
This repository has been archived by the owner on Aug 17, 2022. It is now read-only.

Bug Fix for v0.8 feature configureable hystrix bundle #4

Merged
merged 2 commits into from Aug 14, 2017

Conversation

rajuGT
Copy link
Contributor

@rajuGT rajuGT commented Aug 13, 2017

Bug Fix: To make use of feature released in v0.8 Configureable hystrix
bundle, the constructor of the bundle should be public to extend the
class. Looks like the bundler is always configured using the Builder
and this issue has never occurred/been reported.

Also this has been missed out in unit testing PR#3. As the test class is
under the same package this was unnoticed during that time.

bundle, the constructor of the bundle should be public to extend the
class. Looks like the bundler is always configured using the Builder
and this issue has never occurred/been reported.

Also this has been missed out in unit testing PR#3. As the test class is
under the same package this was unnoticed during that time.
@coveralls
Copy link

Coverage Status

Coverage decreased (-10.9%) to 89.13% when pulling a2a7593 on rajuGT:bug_fix_configurable_hystrix_bundle into fd4a480 on zapodot:master.

@rajuGT rajuGT mentioned this pull request Aug 13, 2017
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 83ae0b2 on rajuGT:bug_fix_configurable_hystrix_bundle into fd4a480 on zapodot:master.

@zapodot zapodot merged commit 2821dc7 into zapodot:master Aug 14, 2017
@zapodot
Copy link
Owner

zapodot commented Aug 15, 2017

Hi!

Moved the ability to override the metrics publisher the setting to the builder. You can now use a Lambda expression to decide whether metrics should be published or not.
Example:
HystrixBundle.builder().withMetricsPublisherPredicate(() -> true).build();

See also the test at https://github.com/zapodot/hystrix-dropwizard-bundle/blob/master/src/test/java/org/zapodot/hystrix/bundle/HystrixBundleTest.java#L114

This implementation will be published in version 0.9 (as 0.8 is kind of defunct)

@rajuGT
Copy link
Contributor Author

rajuGT commented Aug 15, 2017

Hi @zapodot,

The main intention of PRs #3 and #4 was to decide hystrix metric publishing using config file i.e. using environment. Meaning if I've my config file myapp.yml and a field which decides for this may be like enableHystrixMetricsPublisher, I can give value to it say true or false so that I can enable and disable this feature without any code change.

With the new changes v0.9, in dropwizard's initialize block there is no access for environment. So using the previous approach ConfiguredBundle we can get access of environment variable and we can achieve the toggling feature.

@zapodot
Copy link
Owner

zapodot commented Aug 15, 2017

Sure! No problem we just add it to the functional interface MetricsPublishPredicate so that it is provided to your lamda function. Something like:
HystrixBundle.builder().withMetricsPublisherPredicate((c) -> c.isPublishEnabled()).build() where the input is the configuration POJO of your application. Shouldn't that solve your issue?

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

Successfully merging this pull request may close these issues.

None yet

3 participants