Add Action Cable to rails/master #22585

dhh · 2015-12-14T15:39:52Z

It Is Time.

… callbacks

@lifo

@lifo I couldn’t find any use of documentation for this, so removed it for now. Was it just for testing?

…nsubscribe

We have the remote connections to immediately cut a connection when someone has been kicked off. Let’s lean on that for now.

fix typo

Update README find_verified_user example

…to_readme README.md instructions for configuring allowed request origins

Handle cases where logger is not a tagged logger.

Fix an error when using multiple gid identifiers

…mode Add logging to stdout in development mode.

@ascrazy

Thanks to feedback from @ascrazy, @jeremy, and @javan

Allow regexp for a allowed_request_origins array

Easy websocket url configuration

add devise example to readme

Freshen up the client-side subscription examples. Fixes #118

sgrif · 2015-12-14T15:43:17Z

Gemfile.lock

@@ -361,6 +390,7 @@ DEPENDENCIES
  rails!
  rake (>= 10.3)
  redcarpet (~> 3.2.3)
+  redis (~> 3.0)


Since we never solved this problem, what do you think about having actioncable be in the default Gemfile instead? We probably shouldn't have the rails gem have a hard dependency on redis

We should make redis "optional" in the same way that mysql or postgresql is optional for activerecord. So a run-time requirement. But Action Cable ships with Rails by standard, not in the Gemfile.

I'm not sure that'll work here, since we conditionally require the adapter based on the configuration, but Action Cable will always end up being required. I'm concerned that this will cause a large number of existing apps to be unable to upgrade by default, if Redis isn't already installed on the machine.

I think the redis requirement can just kick in when we start the server
with ./bin/cable or whatever. And then the redis gem file can be listed by
default in the default Gemfile for new apps. And if someone tries to start
bin/cable without that, we'll complain about why.

On Mon, Dec 14, 2015 at 4:57 PM, Sean Griffin notifications@github.com
wrote:

In Gemfile.lock
#22585 (comment):

@@ -361,6 +390,7 @@ DEPENDENCIES
rails!
rake (>= 10.3)
redcarpet (~> 3.2.3)

redis (~> 3.0)

I'm not sure that'll work here, since we conditionally require the adapter
based on the configuration, but Action Cable will always end up being
required. I'm concerned that this will cause a large number of existing
apps to be unable to upgrade by default, if Redis isn't already installed
on the machine.

—
Reply to this email directly or view it on GitHub
https://github.com/rails/rails/pull/22585/files#r47514517.

@dhh I think this would work. Will this have any after effects when someone is trying to upgrade from 4.2.x to 5?

Cool. I think whatever strategy we apply for redis should also be used for eventmachine as it's pretty heavyweight as well. We should see if we can drop the dependency on coffee-rails as well, as that's not something that rails currently has a dependency.

nynhex · 2015-12-14T15:50:40Z

Massive and amazing!

claudiob · 2015-12-14T16:06:46Z

actioncable/README.md

+like Unicorn, but the latter must be multi-threaded, like Puma.
+
+
+## Alpha disclaimer


@dhh Some sections of this README need to be revised for ActionCable to become part of Rails (such as this Alpha disclaimer).

Would you like me to open a PR against your branch with the suggested changes?
Should I also truncate the lines of the README following Rails conventions?

rafaelfranca · 2015-12-14T16:08:33Z

We need a railtie for action cable. We need to change rails/all to load this new railtie. We will also need a skip_actioncable option and to update the application.rb to take in consideration that.

All these missing things will address the @sgrif comment about redis being required.

rafaelfranca · 2015-12-14T16:09:55Z

Gemfile

@@ -64,6 +64,17 @@ group :job do
  gem 'sequel', require: false
 end

+# Action Cable
+group :cable do
+  gem 'faye-websocket',   '~> 0.10.0', require: false


We can remove all these dependencies but puma from here. They are all in the gemspec.

dhh · 2015-12-14T16:20:22Z

Closed in favor of #22586.

rafaelfranca · 2015-12-14T16:23:20Z

actioncable/lib/action_cable/engine.rb

+    config.action_cable = ActiveSupport::OrderedOptions.new
+
+    config.to_prepare do
+      ApplicationController.helper ActionCable::Helpers::ActionCableHelper


Should we include in ActionView::Base?

Yes, that makes more sense. Should be available everywhere.

On Mon, Dec 14, 2015 at 5:24 PM, Rafael França notifications@github.com
wrote:

In actioncable/lib/action_cable/engine.rb
#22585 (comment):

@@ -0,0 +1,27 @@
+require 'rails/engine'
+require 'active_support/ordered_options'
+require 'action_cable/helpers/action_cable_helper'
+
+module ActionCable

class Engine < ::Rails::Engine

config.action_cable = ActiveSupport::OrderedOptions.new

config.to_prepare do

ApplicationController.helper ActionCable::Helpers::ActionCableHelper

Should we include in ActionView::Base?

—
Reply to this email directly or view it on GitHub
https://github.com/rails/rails/pull/22585/files#r47518344.

dhh added 30 commits July 7, 2015 19:03

Use latest Bundler

4bb995a

Refer to the proper logger

4c0ece2

Bundle connect/disconnect callbacks together with all other subscribe…

53c4b41

… callbacks

Extract periodic timers concern

e04c95e

Style

70361c7

Removing unused matches? method

e6effb2

@lifo I couldn’t find any use of documentation for this, so removed it for now. Was it just for testing?

Don't need a delegator for a single-use case

6fe8a87

Spacing

ab77cb7

Switch domain language from channel connect/disconnect to subscribe/u…

d2a3598

…nsubscribe

No need to use a channel_ prefix inside the channel

569b751

No need for this to be public

982a488

Go with process_action to match language from Action Controller

a21f6c8

Starting the documentation process

0ca0747

Expand authors given recent work

35e6de4

Use process vs perform language

9b254fa

Add documentation

a01317e

Remove the authorized check for now

35ffec2

We have the remote connections to immediately cut a connection when someone has been kicked off. Let’s lean on that for now.

No need for this delegator

6f4e9de

Allow actions not to accept the data argument

74d764b

Explain action processing

5acf45b

Spacing

b78e960

Document the remaining parts of the Channel setup.

81bbf9e

Basic authentication helpers

049cd82

Identifiers will add attr_accessor as well for convenience

060284f

Document the connection

a2c08e7

Document heartbeat purpose

e3bf826

Document and make private method private

7bcc0e4

Make the entire internal channel private

338e28d

More clear method name for what's actually happening

65033ba

Documentation

7333feb

rafaelfranca and others added 19 commits December 3, 2015 13:02

Merge pull request #128 from ma2gedev/patch1

928aed6

fix typo

Update README find_verified_user example

a3d293b

Merge pull request #133 from assimovt/master

0e283fe

Update README find_verified_user example

Merge pull request #126 from mikecarroll/add_allowed_request_origins_…

2b4aa9a

…to_readme README.md instructions for configuring allowed request origins

Merge pull request #125 from lsylvester/handle-custom-loggers

cf4d9aa

Handle cases where logger is not a tagged logger.

Merge pull request #115 from habermann24/master

a86d506

Fix an error when using multiple gid identifiers

Merge pull request #57 from lsylvester/log-to-stdout-in-developement-…

c362bea

…mode Add logging to stdout in development mode.

add devise example to readme

3816fac

Freshen up the client-side subscription examples. Fixes #118

14628ff

Associate comments with perform

2b33675

Easy websocket url configuration

62250e3

Thanks to feedback from @ascrazy, @jeremy, and @javan

Allow regexp for a allowed_request_origins array

09e10ef

Remove unused method allowed_origins in Connection::Base

1c6fb5e

Merge pull request #130 from adamliesko/allow_regexps_allowed_origins

5fec4b9

Allow regexp for a allowed_request_origins array

Merge pull request #73 from smellsblue/websocket-url-differentiation

7328aa4

Easy websocket url configuration

Merge pull request #135 from gregmolnar/readme

a890c81

add devise example to readme

Merge pull request #137 from rails/freshen-client-side-examples

4073a3e

Freshen up the client-side subscription examples. Fixes #118

Get ready to merge into Rails

bf40bdd

Initial stab at adding Action Cable to rails/master

760de78

sgrif reviewed Dec 14, 2015
View reviewed changes

claudiob reviewed Dec 14, 2015
View reviewed changes

rafaelfranca reviewed Dec 14, 2015
View reviewed changes

dhh closed this Dec 14, 2015

rafaelfranca reviewed Dec 14, 2015
View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Action Cable to rails/master #22585

Add Action Cable to rails/master #22585

dhh commented Dec 14, 2015

sgrif Dec 14, 2015

dhh Dec 14, 2015

sgrif Dec 14, 2015

dhh Dec 14, 2015

nynhex Dec 14, 2015

sgrif Dec 14, 2015

nynhex commented Dec 14, 2015

claudiob Dec 14, 2015

rafaelfranca commented Dec 14, 2015

rafaelfranca Dec 14, 2015

dhh commented Dec 14, 2015

rafaelfranca Dec 14, 2015

dhh Dec 15, 2015

		like Unicorn, but the latter must be multi-threaded, like Puma.


		## Alpha disclaimer

Add Action Cable to rails/master #22585

Add Action Cable to rails/master #22585

Conversation

dhh commented Dec 14, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nynhex commented Dec 14, 2015

Choose a reason for hiding this comment

rafaelfranca commented Dec 14, 2015

Choose a reason for hiding this comment

dhh commented Dec 14, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment