Basic implementation

[sadovnik]

This is a basic implementation of concept of running RPC-server with the tests within single process. There are four simple tests that are implemented and tested on anycable-go 0.3.0. I need to get some feedback to understand am I going on the right direction or not. : )

[palkan]

Why did you choose docopt? We can use OptionParse, which is a common practice. Also we can avoid the additional dependency.

[sadovnik]

I think that declarative approach is significant when it comes to building user interfaces. That's why I choose Docopt. It requires you to specify the actual help message that will be displayed to the end user to declare argument parsing rules.

Also we can avoid the additional dependency.

Docopt is relatively compact (16KB) and it has well-established API.

We can use OptionParse, which is a common practice.

Why is this important? Could you please expand on this?

[palkan]

• I don't know any project that uses it

• A lot of popular Ruby projects use OptionParser (Sidekiq, Rubocop to name a few)

• It has not been maintained for a rather long time (https://ossert.evilmartians.io/docopt)

[sadovnik]

I don't know any project that uses it

A lot of popular Ruby projects use OptionParser (Sidekiq, Rubocop to name a few)

That's true: https://rubygems.org/gems/docopt/reverse_dependencies

It has not been maintained for a rather long time

https://ossert.evilmartians.io/docopt: Active Years Age: 4+ years

🤔

[palkan]

"Active Years Age" takes into an account issues and PRs. Yep, kind of confusing.

For the other hand, the commits history shows the real situation. As well as the state of open issues and PRs.

[sclinede]

BTW the main Ossert application is to aggregate several metrics to come up to a more balanced grade of maintenance quality. So it is not very good to use one isolated metric to make your decision.

Anyway I'll try to find better name for "Active Years Age" in order to frustrate nobody.

[palkan]

Let's move debug option into a general Anycablebility config.

We can also extract other options into a config and use it everywhere (e.g. Anycablebility.config.redis_url, etc).

[sadovnik]

We can also extract other options into a config and use it everywhere (e.g. Anycablebility.config.redis_url, etc).

Should we use Anyway for this?

[palkan]

Yep, it's already here.

[sadovnik]

I don't really understand what's the benefit of using Anyway for such task. I mean who will want to use Anycablebility inside a Rails app or configure it via environment variables?

I'm also not sure why should we introduce global state here instead of just passing config around. This makes no sense to represent our components as objects if they will rely on some global state, isn't it?

[palkan]

Agree. Command line arguments would be enough.

Let's introduce a global Logger instead (say, Anycablebility.logger). Or we can even use Anycable.logger, btw.

[sadovnik]

Let's introduce a global Logger instead (say, Anycablebility.logger). Or we can even use Anycable.logger, btw.

Could you please explain what goal do we want achieve with it? We want to config logger once, instead of configuring it in every component separately?

[palkan]

We want to config logger once, instead of configuring it in every component separately?

Exactly.

[sadovnik]

@palkan does the last commit satisfies this requirement?

[palkan]

20 seconds? Maybe, 5 could be enough?

[sadovnik]

Yup. Also, maybe we should make it configurable?

[palkan]

It's better to use IO::NULL instead of '/dev/null'

[palkan]

The purpose of the global/shared logger is to get rid of these parameters: debug, logger.

Let's do the following:

• introduce a Logging module:

module Anycablebility
  module Logging
    PREFIX = 'AC'
    
    class << self
      attr_writer :logger
    end

    def logger
      Anycablebility.logger
    end
    
   # helper method
   # later we'll be able to add a colorising here, depending on the level
    def log(level, message)
      return unless logger
      logger.send(level.to_sym, PREFIX) { message }
    end
  end
end

• Then in #run method:

Anycablebility.logger = Logger.new(STDOUT).tap do |logger|
  logger.level = debug ? Logger::DEBUG : Logger::WARN
end

• When configuring AnyCable:

Anycable.logger = Anycablebility.logger

• And anywhere you need a logger:

include Logging

[sadovnik]

# helper method
# later we'll be able to add a colorising here, depending on the level
def log(level, message)
  return unless logger
  logger.send(level.to_sym, PREFIX) { message }
end

There's a Logger#formatter=. Maybe we should take advantage of it for that purpose?

[sadovnik]

class << self
  attr_writer :logger
end

Why do we need this? This is an attribute writer on Logging itself. We need an attr_accessor :logger on the Anycablebility instead. Am I missing something?

[palkan]

There's a Logger#formatter=. Maybe we should take advantage of it for that purpose?

Great idea 👍
And we can do that later, let's skip for now.

attr_accessor :logger on the Anycablebility instead

Yep, you're right

[sadovnik]

Done.

Could you please take a look at how I require stuff? I'm not sure I do it the right way. I have PHP background so I require components where do I need them, like PHP programmers use their classes and constants. Is that right paradigm? Or we should make some kind of tree with requires like rubocop does?

[sadovnik]

Although this kind of global logger makes some parts of code less verbose but also less elegant. There's gotta be a practical yet elegant way of doing that.

[palkan]

Could you please take a look at how I require stuff?

LGTM

[palkan]

Let's merge it and I'll play with it for a while and propose future improvements.

[sadovnik]

@palkan let's discuss and list test cases to do.