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

Replace Mochiweb/Webmachine with Cowboy/Cowmachine. #1355

Merged
merged 33 commits into from Aug 30, 2016
Merged

Replace Mochiweb/Webmachine with Cowboy/Cowmachine. #1355

merged 33 commits into from Aug 30, 2016

Conversation

mworrell
Copy link
Member

@mworrell mworrell commented Jul 27, 2016

Fixes #533

See also: https://github.com/zotonic/cowmachine 🐮

  • Remove mochiweb/webmachine http server initialization
  • Add cowboy http server initialization
  • Remove webmachine from deps
  • Replace #wm_reqdata{} with type mochiweb_req:req()
  • Replace calls to wrq with calls to cowmachine_req
  • Update controllers to simpler version with one Context argument
  • Change service API to only use a Context argument
  • Binaryfication of arguments
  • Add web socket upgrade
  • Cookie setting (new code in cowboy2)
  • Get it compiling
  • Test, test, test, and test.

Proposal is to move the re-addittion of the access log (z_access_log, z_stats, z_access_syslog and request watchers) to another separate issue.

@ddeboer
Copy link
Member

ddeboer commented Jul 28, 2016

Yeah, great start! 🐮

@@ -4,6 +4,14 @@
2},
{<<"bert">>,{pkg,<<"bert">>,<<"0.1.0">>},0},
{<<"cf">>,{pkg,<<"cf">>,<<"0.2.1">>},1},
{<<"cowboy">>,
{git,"git://github.com/ninenines/cowboy.git",
Copy link
Member

@arjan arjan Aug 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why no hex.pm dependencies?

(oh wait, different branch, probably)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, we are using the master (2.0 development) version of cowboy.

@mworrell mworrell added this to the 1.0 milestone Aug 26, 2016
@mworrell
Copy link
Member Author

mworrell commented Aug 26, 2016

I propose to add the access logs and stats via a different issue.

/cc @mmzeeman @arjan

@ddeboer
Copy link
Member

ddeboer commented Aug 27, 2016

And it’s green! 👏

@mworrell As you’ve just converted Zotonic’s internal controllers, would you mind adding a section to upgrading.rst on how users can convert any custom controllers they have? Something like:

  1. remove the ReqDate argument
  2. different way of getting request and setting response headers
  3. remove init and ping functions
  4. ...?

@mworrell
Copy link
Member Author

@ddeboer Added docs, also updates some out of date information.
There is also a "Just enough webmachine" chapter, shall we update or remove that?

@mworrell
Copy link
Member Author

We should also check chapters like directory structure that are not updated with the rebar3 changes.

@ddeboer
Copy link
Member

ddeboer commented Aug 29, 2016

Added docs, also updates some out of date information.

Great!

There is also a "Just enough webmachine" chapter, shall we update or remove that?

Let’s remove it.

We should also check chapters like directory structure that are not updated with the rebar3 changes.

As that doesn’t have to do with this change (Cowboy) per se, I created a separate issue for that: #1399.

@mworrell
Copy link
Member Author

Let's warn the mailing list before we merge, as this is an incompatibel and quite drastic change.

* The return value of `generate_etag` must be a binary



Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is a great start. I’ll probably slightly rewrite this later for better developer experience. 😉 I noted that in #1399.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to add an example controller in the documentation, showing all possible callbacks.

@mworrell mworrell changed the title [WIP] Replacing Mochiweb/Webmachine with Cowboy/Cowmachine. Replace Mochiweb/Webmachine with Cowboy/Cowmachine. Aug 29, 2016
@mworrell
Copy link
Member Author

We have warned the mailing list and on twitter, so I think we can merge.

/cc @ArthurClemens @arjan @mmzeeman

@ddeboer
Copy link
Member

ddeboer commented Aug 29, 2016

Let’s give everyone one more day to look at this; if we hear no objections, I’ll squash and merge this tomorrow.

@ddeboer ddeboer merged commit 149aee6 into master Aug 30, 2016
@ddeboer ddeboer deleted the cowboy branch August 30, 2016 14:29
@ddeboer
Copy link
Member

ddeboer commented Aug 30, 2016

BOOM! 💣 🔥 🎆 🚒

@ddeboer ddeboer mentioned this pull request Aug 30, 2016
6 tasks
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

3 participants