search

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

Introducing CORS support. #89

Closed jorgemsrs closed 9 years ago

jorgemsrs commented 9 years ago

This is a minimum viable solution in order to get cors support working in cougar.

It is meant for discussion and to help improve getting to a good enough state to be merged.

jorgemsrs commented 9 years ago

no tests are implemented before we agree on a correct implementation

eswdd commented 9 years ago

Jorge,

If you are able to make the relevant changes in the next month, then we can include this in the Cougar 3.2.0 release.

jorgemsrs commented 9 years ago

Hi @eswdd.

I'll do my best.

jorgemsrs commented 9 years ago

Hello @eswdd .

I've changed the implementation a bit. The build is good but fails because the JettyHttpTransportTest#testCORSEnabledAddNewHandlerToContextHandlerCollection() and JettyHttpTransportTest#testCORSDisabledDoesNotAddHandlersToContextHandlerCollection() use transport.stop() which is painfully slow. Travis does not like it.

I have to dig in and understand why. Do you have any ideas?

UPDATE

Actually the build was broken in fc6dfdd. Need any help?

eswdd commented 9 years ago

That error started after travis upgraded their build environment last week. It would be fixed by #93 which would allow us to reduce the level of testing. For now i'm just rerunning builds that fail and it normally works fine the second time. Have rerun yours and the previous build. For some reason someone had disabled the travis builds. Was that you @betfairframeworks ?

jorgemsrs commented 9 years ago

Having builds working is the minimum state of affairs for this stream of work to continue. I seem unable to kick a build and I reckon it is because someone has disabled them like @eswdd has said. Even if it's not please re-enable them asap.

eswdd commented 9 years ago

I reenabled them last night. Will investigate... On 19 Dec 2014 12:05, "Jorge Silva" notifications@github.com wrote:

Having builds working is the minimum state of affairs for this stream of work to continue. I seem unable to kick a build and I reckon it is because someone has disabled them like @eswdd https://github.com/eswdd. Even if it's not please re-enable them.

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

jorgemsrs commented 9 years ago

Thanks @eswdd . I cannot kick a build anyway. Is is expectable for me to be able to do so?

eswdd commented 9 years ago

I don't think you can kick builds unless you're a committer. Will re-kick yours in a bit.

eswdd commented 9 years ago

Doesn't help that I just merged some changes to fix builds on java8 which had unintended consequences for java7 builds - i can't fix that until this evening.

although the last build on your pull request worked (if you ignore java8)

jorgemsrs commented 9 years ago

Fair enough.

I'll write the doco gist then

eswdd commented 9 years ago

cool

jorgemsrs commented 9 years ago

Here's the gist https://gist.github.com/jorgemsrs/ef67b48f9f927ec2fe2f