search

yammer / breakerbox

Frontend for Tenacity + Archaius
Apache License 2.0
63 stars 29 forks source link

Unable to configure cluster instances with additional URI path #6

Closed rlewan closed 9 years ago

rlewan commented 9 years ago

We have a service which utilizes Tenacity which we would like to connect to a Breakerbox instance. The problem is that the service exposes its Tenacity resources under an additional context, so they are not where Breakerbox expects them to be by default:

some-service-inst1:1234/resilience/tenacity/propertykeys some-service-inst1:1234/resilience/tenacity/configuration/{key} some-service-inst1:1234/resilience/tenacity/circuitbreakers ...

We thought that in such scenario it would be enough to configure the cluster instances appropriately:

  turbine.ConfigPropertyBasedDiscovery.some-service.instances=\
    some-service-inst1:1234/resilience

However, Breakerbox will strip the path part from provided entries and thus attempt to connect to a resource which does not exist. My questions now are:

Any info will be appreciated.

Thanks, Rafal

rlewan commented 9 years ago

PS. I've cloned the project and adjusted code to use configured path (Instances#toPropertyKeyUri). Two Instances class unit tests broke, but after I've force built and launched the Breakerbox it appeared to be functional (this is the reason for my second question).

chrisgray commented 9 years ago

Hello!

The configuration for these instances is very fragile so I apologize for the headaches they are causing you. I'll look into changing the Instances#toPropertyKeyUri call so that others can utilize this as well. There is no particular reason why we would not want you to be able to put these resources wherever you'd like.

rlewan commented 9 years ago

Thanks for a quick update! Hopefully the fix will make this already awesome application even better :)

Cheers, Rafal

chrisgray commented 9 years ago

You want to get credit for the change? Send me your forked PR :-)

chrisgray commented 9 years ago

Just committed a fix for this which will come out in the next release! Thanks for the heads up

rlewan commented 9 years ago

Haven't noticed the previous comment, doh ;)

Thanks for the fix!