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
Initial minion implementation #20
Conversation
surprisingly enough
Ready for review. I ended up giving the README some love as well and adding the test coverage badge (why not?). This would close #9 I think as that doesn't really put any requirements on what the agent does other than connect to the master. The minion node should automatically connect to the configured master node when it is available, both sides detect disconnects and should automatically reconnect when possible. I opted for a fairly simple distribution scheme here - nothing fancy like Phoenix.Presence or PubSub. That may be something good to do eventually - especially if we want the "master" to be clusterable - but I'd rather let the requirements sort themselves out before trying to implement anything. |
Medera is a [Slack](https://slack.com) bot with _life goals_. | ||
|
||
Medera is a project of the Rochester, NY, | ||
[functional programming user group](https://www.meetup.com/%CE%BB-Rochester-Functional-Programming-Language-Meetup/). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lol, this link might be changing soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weeelll then you can make a PR to change it ;P
@@ -39,4 +28,30 @@ defmodule Medera do | |||
Endpoint.config_change(changed, removed) | |||
:ok | |||
end | |||
|
|||
def child_specs(true) do | |||
slack_children() ++ web_children() ++ minion_children() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any applications that are started, but have no use in some situations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I think there would be some applications started (Phoenix, Ecto, etc). I think the minion would need some refinement in how it's packaged before this would be something easy to deploy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note I mean OTP applications whereas your commented is on the child specs for the supervisor... there aren't any OTP children started here that aren't used.
@spec master_node :: atom | ||
def master_node do | ||
case Application.get_env(:medera, :master_node) do | ||
name when is_binary(name) -> String.to_atom(name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to_existing_atom
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is one of those cases where you need to_atom
since there's no reason the master node's name would exist as an atom before this is called.
end | ||
|
||
def init(_) do | ||
{:ok, :disconnected, 10} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the 10
, 100
, and 0
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are timeouts - see https://hexdocs.pm/elixir/GenServer.html#c:handle_call/3 . In this particular example, after 10 milliseconds the GenServer will receive a :timeout
message which gets handled by handle_info(:timeout, ...)
below. It's a way to basically implement a state machine (see also http://erlang.org/doc/man/gen_fsm.html).
I should make module attributes to give them names though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so it's like an asynchronous sleep and call? Clever.
I want to add some documentation before merging. Opening PR partly for self-review.