Use monad for handling errors and general flow, also config file #1
Conversation
…> write_to_client
JackyChiu
left a comment
JackyChiu
left a comment
There was a problem hiding this comment.
Few comments about how the config should be set/read
|
|
||
| config :active_proxy, | ||
| timeout: 100, | ||
| node1_address: "159.203.44.11" |
There was a problem hiding this comment.
This should be a list key instead of a indexed key lol
upstream_hosts: [
"159.203.44.11"
]
There was a problem hiding this comment.
I think we talked about change address during runtime so I assumed named addresses would be best. Not sure if we still want to do that.
There was a problem hiding this comment.
Did we decide that we didn't want dynamic changes to the upstream hosts?
There was a problem hiding this comment.
I honestly don't remember.
There was a problem hiding this comment.
Proposal for actual storage: store active/inactive data in a persistent KV store instead because it's more flexible. Imagine the case where we decide to have dynamic upstream being added and the proxy restarts. That state is lost and the proxy goes back to relying on the static config. Whereas with the KV store we'd have that state saved. Obviously, this is out of scope for this PR but we should consider this for future active/inactive storage.
For this PR I say you just make a decision of a list or a named active/inactive hosts.
|
|
||
| defp start_serve_process(socket) do | ||
| timeout = Application.get_env(:active_proxy, :timeout) | ||
| application_address = Application.get_env(:active_proxy, :node1_address) |
There was a problem hiding this comment.
Can you look into if we can have these as constants on the module? and for the module to eval them on startup?
Otherwise, an init on the module should accept a config object and they should be set as constants there
| defp start_serve_process(socket) do | ||
| timeout = Application.get_env(:active_proxy, :timeout) | ||
| application_address = Application.get_env(:active_proxy, :node1_address) | ||
| Logger.info("Forwarding to application at #{application_address} with timout of #{timeout}ms") |
There was a problem hiding this comment.
Move this log to where that goes as well ^
| :ok <- :gen_tcp.send(upstream_socket, packet), | ||
| {:ok, packet} <- :gen_tcp.recv(upstream_socket, 0, timeout), | ||
| :ok <- :gen_tcp.send(client_socket, packet) do | ||
| serve(client_socket, upstream_socket, timeout) |
There was a problem hiding this comment.
Print this function out and hang it on a wall 😍
What this PR does