betfair / cougar

Cougar is a framework for making building network exposed service interfaces easy.
http://betfair.github.io/cougar
Apache License 2.0
27 stars 18 forks source link

#84 Fixing calculation of TimeConstraints based on timeouts for Async clients #85

Closed andredasilvapinto closed 9 years ago

andredasilvapinto commented 9 years ago

https://github.com/betfair/cougar/issues/84

eswdd commented 9 years ago

Couldn't remember if this was intentional, after reviewing the original issue and code I can't see a reason why it would be.

andredasilvapinto commented 9 years ago

I see. Disallowing requests without timeout (or only giving the option to do so on a per-application basis) is probably a sensible option, but imo that would need to be done also on the sync client and have a more adequate error message like an IllegalArgumentException saying "You need to specify a timeout" or "Requests without timeout are not allowed: timeout needs to be greater than 0".

eswdd commented 9 years ago

Requests without timeouts are allowed (currently), although servers may choose to add a default. Certainly no requirement for that validation came with the original reqs, so we can add if needed.

On 28 Oct 2014, at 22:23, André Pinto notifications@github.com wrote:

I see. Disallowing requests without timeout (or only giving the option to do so on a per-application basis) is probably a sensible option, but imo that would need to be done also on the sync client and have a more adequate error message like an IllegalArgumentException saying "You need to specify a timeout" or "Requests without timeout are not allowed: timeout needs to be greater than 0".

— Reply to this email directly or view it on GitHub https://github.com/betfair/cougar/pull/85#issuecomment-60842480.

andredasilvapinto commented 9 years ago

Cool. I was just saying that I understood why you thought that it might have been intentional as I can see some value on it. I'm not really requesting a "mandatory-timeout" feature. Right now I still think it is OK to allow for requests without timeout and give the responsibility for this decision to the Cougar users.

andredasilvapinto commented 9 years ago

On another note, could we get a patched release of Cougar with this fix so we can use it on the services without having to do an unofficial patch?

eswdd commented 9 years ago

Which version? 3.1?

On 28 Oct 2014, at 22:37, André Pinto notifications@github.com wrote:

On another note, could we get a patched release of Cougar with this fix so we can use it on the services without having to do an unofficial patch?

— Reply to this email directly or view it on GitHub https://github.com/betfair/cougar/pull/85#issuecomment-60844122.

andredasilvapinto commented 9 years ago

3.1.0 is the one with the bug. The patch release would probably be 3.1.1. On Oct 29, 2014 6:06 AM, "Simon Matic Langford" notifications@github.com wrote:

Which version? 3.1?

On 28 Oct 2014, at 22:37, André Pinto notifications@github.com wrote:

On another note, could we get a patched release of Cougar with this fix so we can use it on the services without having to do an unofficial patch?

— Reply to this email directly or view it on GitHub < https://github.com/betfair/cougar/pull/85#issuecomment-60844122>.

— Reply to this email directly or view it on GitHub https://github.com/betfair/cougar/pull/85#issuecomment-60878325.