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.

Sign up for GitHub

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

Jump to bottom

Generators #110

Merged

bibendi merged 6 commits into 1.0-dev from feature/generators

Nov 5, 2019

Member

bibendi commented Oct 30, 2019 •

edited

Loading

What is the purpose of this pull request?

We want to add an ability to configure any_cable quickly.

What changes did you make? (overview)

Added a "local" generator
Added a "docker" generator
Make texts better

Checklist

Add tests
Add Changelog entry
Update documentation for this change

bibendi force-pushed the feature/generators branch 3 times, most recently from 92c7f33 to a8afd29 Compare

October 31, 2019 10:42

bibendi marked this pull request as ready for review

October 31, 2019 10:43

bibendi requested review from palkan and sponomarev

October 31, 2019 10:43

palkan requested changes

View reviewed changes

Member

palkan left a comment

Looks good 👍

Left a bunch of comments.

A question regarding local installation.
We discussed a little bit different scenario: downloading a binary into the gem's source folder and adding a bin/anycable-go wrapper script to run it.
That would allow users to download binaries on different platforms but run using the same command.

And we can extract downloading into a separate task and allow users to run it separately (rake anycable:download:anycable-go). That could simplify installing anycable-go for local development (and other envs, e.g., CI).

lib/rails/generators/anycable/setup/docker/docker_generator.rb Outdated

+                module Setup
+                  # Generator to help set up AnyCable in Docker environment
+                  class DockerGenerator < ::Rails::Generators::Base
+                    namespace "anycable:setup:docker"

Member

palkan Oct 31, 2019

Let's put these scripting into anycable:setup:dev:... namespace, 'cause it might be confused with production configuration.

Member Author

bibendi Oct 31, 2019

First of all, no one in their right mind won't execute this generator in production.
Also, almost nothing stops to use these AnyCable Compose services in production when deploying to Docker Swarm.

lib/rails/generators/anycable/setup/docker/docker_generator.rb Outdated

+                      say "Docker configs are not standardized."
+                      say "So, right now, we cannot continue installation process."
+                      say "🛈 But, you can find an example of workable configs here: "
+                      say "  👉 https://github.com/evilmartians/evil_chat/tree/feature/anycable"

Member

palkan Oct 31, 2019

Let's add a docker-compose.yml snippet instead for now:

anycable-ws:
  image: anycable/anycable-go:v0.6.4
  ports:
    - '3334:3334'
  environment:
    PORT: 3334
    REDIS_URL: redis://redis:6379/0
    ANYCABLE_RPC_HOST: anycable-rpc:50051
  depends_on:
    - anycable-rpc
    - redis

anycable-rpc:
  <<: *backend
  command: bundle exec anycable   
  ports:
    - '50051'

We will replace it with the link to the documentation later.

lib/rails/generators/anycable/setup/local/local_generator.rb Outdated

+                      end
+                      environment(nil, env: :production) do
+                        'config.action_cable.url = ENV["CABLE_URL"].presence'

Member

palkan Oct 31, 2019

Maybe, ENV.fetch("CABLE_URL")? To make it fail on boot if no env configured.

Or even ENV.fetch("CABLE_URL") { raise "Please, add CABLE_URL env variable pointing to your AnyCable-Go server" }

Member Author

bibendi Oct 31, 2019

Before I wanted, but later I realized that I cannot create a Heroku review app, because it disallows to fill in an empty environment variable. And we want use the async adapter with empty url.

lib/rails/generators/anycable/setup/local/local_generator.rb Outdated

+                    def cable_url
+                      environment(nil, env: :development) do
+                        'config.action_cable.url = ENV.fetch("CABLE_URL", "ws://localhost:3334/cable").presence'

Member

palkan Oct 31, 2019

Let's add a comment to this line (and the one below): "Specify AnyCable WebSocket server URL to make JS client connect to it"

lib/rails/generators/anycable/setup/local/local_generator.rb Outdated

+                      end
+                    end
+                    def heroku

Member

palkan Oct 31, 2019

What about https://github.com/anycable/capistrano-anycable?

Maybe, we can add two separate generators: anycable:setup:deployment:heroku and anycable:setup:deployment:capistrano?

And shouldn't it be a part of the base setup generator?

Member Author

bibendi Oct 31, 2019

Sure, we can describe all deployment methods that we know =)
But for the first stage, I choose only two methods. Will do in a separate PR.

part of the base setup generator?

Agree

lib/rails/generators/anycable/setup/local/local_generator.rb Outdated

+                    end
+                    def configs
+                      inside("config") { template "cable.yml" }

Member

palkan Oct 31, 2019

Can we just change the adapter setting? Everything else is not relevant to AnyCable, let's leave as is.

Member Author

bibendi Oct 31, 2019

We could try, but there is a chance to fail.
It is better to promote our right config, then a developer can view the diff and merge it by a mergetool. This functionality give Thor out of the box.

lib/rails/generators/anycable/setup/local/local_generator.rb Outdated

+                      say_status :info, "✔ AnyCable-Go WebSocket server has been installed"
+                    end
+                    def configs

Member

palkan Oct 31, 2019

Let's add config/anycable.yml as well.

We already had it in the previous version: https://github.com/anycable/anycable-rails/blob/v0.5.5/lib/generators/anycable/templates/anycable.yml (it contains a lot of useful annotations).
It's a bit outdated (e.g., it doesn't contain access_logs_disabled: false parameter).

Member Author

bibendi Oct 31, 2019

Hm, I don't know about it. Before I need to place it at this PR evilmartians/evil_chat#5 and I need to figure out what to do with hardcoded rpc_host and redis_url options

lib/rails/generators/anycable/setup/local/local_generator.rb Outdated

+                      say_status :info, "✔ AnyCable-Go WebSocket server has been installed"
+                    end
+                    def configs

Member

palkan Oct 31, 2019

This should be a part of the setup generator, not setup:local.

lib/rails/generators/anycable/setup/local/local_generator.rb Outdated

+                      inside("config") { template "cable.yml" }
+                    end
+                    def cable_url

Member

palkan Oct 31, 2019

This should be a part of the setup generator, not setup:local.

lib/rails/generators/anycable/setup/local/local_generator.rb Outdated

+                      say_status :help, "🛈 You can read more Heroky deployment here 👉 https://docs.anycable.io/#/deployment/heroku", :yellow
+                    end
+                    def finish

Member

palkan Oct 31, 2019

setup:local -> setup

Member Author

bibendi commented Oct 31, 2019

We discussed a little bit different scenario: downloading a binary into the gem's source folder and adding a bin/anycable-go wrapper script to run it.

That was only the proposal. I find it's not a good idea. There is no difference where a user places the binary, but it is better and more transparent for a user when the binary would be placed on the usual bin path.

bibendi force-pushed the feature/generators branch from a8afd29 to 099fbf3 Compare

November 1, 2019 07:02


          feat: add generators to set up any_cable

f54ce56

bibendi force-pushed the feature/generators branch from 099fbf3 to f54ce56 Compare

November 1, 2019 07:07

Member Author

bibendi commented Nov 1, 2019

Slightly refactored after the review.


          Update generators verbiage

db070d3

palkan self-requested a review

November 1, 2019 12:50


          chore: remove excess method

d88d4bb

palkan changed the base branch from master to 1.0-dev

November 1, 2019 14:45

palkan and others added 2 commits

November 4, 2019 22:44


          feat: add note about action_cable_meta_tag


          tests: cover setup generator

bibendi force-pushed the feature/generators branch from 7628052 to 8712842 Compare

November 5, 2019 12:45

Member Author

bibendi commented Nov 5, 2019

@palkan can you look

palkan reviewed

View reviewed changes

spec/spec_helper.rb

               Dir["#{__dir__}/support/**/*.rb"].each { |f| require f }
               RSpec.configure do |config|
+                include ActiveSupport::Testing::Stream

Member

palkan Nov 5, 2019

👍


          fix: update generatos verbiage, pt 2

f392025

palkan approved these changes

View reviewed changes

Member

palkan left a comment

Looks good to me!
Thanks!

bibendi commented

View reviewed changes

lib/rails/generators/anycable/setup/setup_generator.rb

+                  source_root File.expand_path("templates", __dir__)
+                  METHODS = %w[skip local docker].freeze
+                  SERVER_VERSION = "v0.6.4"

Member Author

bibendi Nov 5, 2019

We definitely need to not use a server version in a constant. Otherwise, we will forget to change that constant every time after the server was released.
There is a secret url that can get the latest asset from the latest release.
https://github.com/anycable/anycable-go/releases/latest/download/anycable-go-v0.6.4-linux-amd64
The big problem we have is that a binary contains a version in a name. This is not the necessary information. I propose to cut it.

Member Author

bibendi Nov 5, 2019

@palkan What do think? Can we roll out a new release with new naming to have the ability to change the version to the latest here?

Member

palkan Nov 5, 2019 •

edited

Loading

Yeah, that's a good idea. Let's add a ticket to the v1.0 project.

For now, let's leave as is (let's add a TODO: or FIXME: with this comment), so we'll be able to test the generator without waiting for anycable-go fixes.

bibendi merged commit 3fcb3b1 into 1.0-dev

bibendi deleted the feature/generators branch

November 5, 2019 18:24

palkan pushed a commit that referenced this pull request


          Generators (#110)

42e611d

feat: add a setup generator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet