support srv cluster queries #16
Conversation
|
FYI: #8 |
Oh, thanks totally missed that issue. I think I searched for 'svr' instead of 'srv'. I made that mistake a couple of times already today :D |
|
I added a second commit to make the dns_rr_type configurable |
lib/dns_cluster.ex
Outdated
| |> Enum.map(fn {_prio, _weight, _port, host_name} -> | ||
| case :inet.gethostbyname(host_name) do | ||
| {:ok, hostent(h_addr_list: addr_list)} -> addr_list | ||
| {:error, _} -> [] | ||
| end | ||
| end) | ||
| |> List.flatten() |
lib/dns_cluster.ex
Outdated
| if valid_query?(query) do | ||
| with( | ||
| {:valid_query?, true} <- {:valid_query?, valid_query?(query)}, | ||
| {:valid_rr_types?, true} <- {:valid_rr_types?, valid_rr_types?(dns_rr_types)} |
There was a problem hiding this comment.
Let's write those as regular conditions instead:
if not valid_query?(query) do
raise ...
end
if not valid_rr_type?(...) do
raise ...
end
This way there is no need for with and tagged tuples.
lib/dns_cluster.ex
Outdated
| max_concurrency: max(1, length(ips)), | ||
| timeout: timeout | ||
| ) | ||
| |> Stream.run() |
There was a problem hiding this comment.
Unrelated change, please revert it to keep history clean.
lib/dns_cluster.ex
Outdated
| MapSet.new(dns_rr_types) | ||
| |> MapSet.subset?(MapSet.new(@valid_rr_types)) |
There was a problem hiding this comment.
| MapSet.new(dns_rr_types) | |
| |> MapSet.subset?(MapSet.new(@valid_rr_types)) | |
| Enum.all?(dns_rr_types, & &1 in @valid_rr_types) |
There was a problem hiding this comment.
or:
| MapSet.new(dns_rr_types) | |
| |> MapSet.subset?(MapSet.new(@valid_rr_types)) | |
| (dns_rr_types -- @valid_rr_types) == [] |
nduitz
force-pushed
the
support-srv-dns-queries
branch
from
d1c8682 to
eb703dd
Compare
|
Thanks for your feedback. I removed the unrelated change with a fix up so it's gone :) |
lib/dns_cluster.ex
Outdated
| {:error, _} -> [] | ||
| end | ||
| end) | ||
| |> Enum.filter(& &1) |
There was a problem hiding this comment.
Can an entry in h_addr_list ever be nil or false? :) Or can we remove this filter?
There was a problem hiding this comment.
Ah true, thanks I had the error case return nil before. That filter does not make sense anymore.
lib/dns_cluster.ex
Outdated
| @@ -70,6 +91,8 @@ defmodule DNSCluster do | |||
| `"myapp.internal"` or `["foo.internal", "bar.internal"]`. If the basename | |||
| differs between nodes, a tuple of `{basename, query}` can be provided as well. | |||
| The value `:ignore` can be used to ignore starting the DNSCluster. | |||
| * `:dns_rr_types` - the resource records types that are used for node discovery. | |||
There was a problem hiding this comment.
perhaps we should call it resource_types?
There was a problem hiding this comment.
Especially because dns is somewhat implied? and rr may not be clear enough
There was a problem hiding this comment.
My intention was to mirror the name of the type in erlang:
https://www.erlang.org/doc/apps/kernel/inet_res.html#t:dns_rr_type/0
But I am happy to change the name to resource_types
There was a problem hiding this comment.
I would say that as an implementation detail, I think something higher level would be better! Thank you!
Hey there.
Due to the infrastracture setup of one of our projects we can only use srv dns records.
This addition should support them besides A / AAAA records.
Let me know if you need further adjustments :)