Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide extra callbacks for cowboy protocol 1,2 #22

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

define-null
Copy link
Contributor

The main motivation of this pullrequest is as following:

If one is using cowboy middleware for example to do the logging of the http body - then the body will be no longer available through cowboy_req:body/2 call. This pullrequest allows to provide Body by using soap_cowboy_1_protocol:upgrade/5 or soap_cowboy_2_protocol:upgrade/7. In this case extra tiny module will be needed from the user's side to read body from whether he stored it, and provide it down to the soap library.

For example in our case middleware that does the logging stores body with cowboy_req:meta/2.

@define-null
Copy link
Contributor Author

@cmullaparthi What do you think?)

@cmullaparthi
Copy link
Contributor

I've had a quick look and it looks good. I want to look a bit deeper before I merge it - will get it done soon. Thanks for the contribution.

@define-null
Copy link
Contributor Author

@cmullaparthi ping me if you want me to provide some tests regarding those callbacks.

@cmullaparthi
Copy link
Contributor

@define-null this looks good to me, but it would be great to have some tests validating this. Could you please provide some?

@define-null
Copy link
Contributor Author

@cmullaparthi Provided tests for the this functionality. Tests on 1.1.x cowboy branch pass fine.

BTW Current cowboy2 integration seems to be broken. At least api that I checked here https://github.com/ninenines/cowboy/blob/2.0.0-pre.5/src/cowboy.erl differs from what soap expect it to be.

And thank you for the library!

@define-null define-null force-pushed the feature/extra-callbacks-for-middleware branch from 38545b2 to 8c1a7c1 Compare January 18, 2017 14:58
@define-null
Copy link
Contributor Author

@define-null
Copy link
Contributor Author

Ping :)

@define-null
Copy link
Contributor Author

@cmullaparthi would be nice to see your comments on this

@define-null
Copy link
Contributor Author

Any updates? We may drop this functionality for cowboy 2.0 for now, untill 2.0.0 final goes live

@define-null
Copy link
Contributor Author

@willemd ping

@define-null
Copy link
Contributor Author

So the cowboy 2.0 is already there
@cmullaparthi @willemdj are you gonna consider this pullrequest if I will introduce all the necessary changes?

Set ibrowse version to point to the latest stable, add logs dir.
@define-null define-null force-pushed the feature/extra-callbacks-for-middleware branch from 8bb49b3 to 8cc6d6b Compare March 28, 2018 14:47
@define-null
Copy link
Contributor Author

I fixed support for cowboy 2. Tested on both cowboy 2.2.2 and cowboy 1.1.2

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

Successfully merging this pull request may close these issues.

None yet

2 participants