Javanica/Support raiseHystrixExceptions for Observables

[michaelcowan]

Adds support for Observable HystrixCommands to optionally raise HystrixRuntimeException.
This extends upon #1397

e.g.

@HystrixCommand(raiseHystrixExceptions = {HystrixException.RUNTIME_EXCEPTION}) public Observable<User> getUserById(final String id) { // ... }

[mattrjacobs]

Thanks for the contribution, @michaelcowan ! This looks reasonable to me, but before I merge, I'd love feedback from those who discussed #1344/#1397 to review.

/cc @dmgcodevil @jorgefr @56quarters @nytins @mukteshkrmishra @hroongta

[dmgcodevil]

@mattrjacobs you already merged the PR for regular commands, nothing new in this PR. BTW: you merged PR with pending comments, in next time could you wait until all comments resolved ?

[michaelcowan]

@dmgcodevil Thats weird - I don't see any unresolved comments on #1397. It shows just myself and @mattrjacobs as the only participants on my screen.

[dmgcodevil]

@michaelcowan I left few comments for sure, no idea why they aren't visible for you. Probably I did something wrong.

[michaelcowan]

@dmgcodevil Is there something I can maybe answer now?

[dmgcodevil]

@michaelcowan why do we need array type for new added property? I thought it's either true or false. Is there an opportunity of extending this in the future ? Is global properties working for the new feature?

[mukteshkrmishra]

@mattrjacobs : Changes looks good to me.

[michaelcowan]

@dmgcodevil I used an array for two reasons, which are actually pretty much your other two questions 😄

1. I wanted to be able to default to null*, so that I could support the global properties. If I defaulted to a false in default properties there would be no way to know if the user explicitly set the value or not.
2. I thought it might be a nice pattern to expand for other Hystrix exception types should it become requested. In particular I was thinking someone might wish to raise HystrixBadRequestException. This would allow Javanica to operate in exactly the same way as hand rolling commands.

(*) You can't default to null on Object types for some reason

"Note that null is not a legal element value for any element type." - JLS

[mattrjacobs]

@michaelcowan Thanks for the PR! Looks good to me.

@dmgcodevil Likewise, I didn't see any outstanding comments. Will wait for a +1 from you to merge this one.

[dmgcodevil]

@mattjacobs No objections from my side. Although there is a question on
stack overflow that might be related to this, I'm looking into it

On Monday, November 14, 2016, Matt Jacobs notifications@github.com wrote:

@michaelcowan https://github.com/michaelcowan Thanks for the PR! Looks
good to me.

@dmgcodevil https://github.com/dmgcodevil Likewise, I didn't see any
outstanding comments. Will wait for a +1 from you to merge this one.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1412 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABWS8QcWxFdnKxSK9nXB3mUJWpXX3eEBks5q-Tl7gaJpZM4Kot39
.

[mattrjacobs]

Any concerns about merging this is, or is there still more to discuss @dmgcodevil ?

[dmgcodevil]

No, go ahead.

…

On Sat, Dec 3, 2016 at 11:18 AM, Matt Jacobs ***@***.***> wrote: Any concerns about merging this is, or is there still more to discuss @dmgcodevil <https://github.com/dmgcodevil> ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1412 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABWS8XaD3nFUVptr_MDxX6SNcrfeGNI4ks5rEZZugaJpZM4Kot39> .

[mattrjacobs]

Thanks @michaelcowan !