-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4437/#review14539
-----------------------------------------------------------
First off, good page! It's pretty easy to follow the expected flow for DNS
queries based on the API you've provided. From a low-level, there isn't much
that's missing: you have ways of performing synchronous and asynchronous DNS
lookups, and you have methods for examining the results. I have a couple of
critiques of the presented API:
* ast_dns_resolve_async_recurring() is not very well defined and there are no
examples illustrating its use.
* The function ast_dns_query_set_get() creates a leaky abstraction for the
query set. I suggest one of the following:
* Have ast_dns_query_set_add() return an integer "token" that can be used
to retrieve that record from the query set. (enthusiasm level: meh)
* Have an ast_dns_query_set iterator API to iterate over the queries in the
set. (enthusiasm level: YES)
* While there are ways of querying if a result is secure, there is no current
way of requesting only secure results.
>From a low-level, the one big thing that I feel is not talked about is the
>threading model. I know that's getting really low level, but there are some
>questions that came to mind, such as: if I perform multiple asynchronous DNS
>lookups (without using a query set) can I guarantee the results will be
>presented to me serially, or can the results be presented in separate threads
>at the same time? This can have an impact on how I write my async callbacks,
>especially if I pass a reference to the same user data to both async queries.
The low-level API looks fine, and it provides a lot of areas where some
higher-level functions could be created. For instance:
* NAPTR has a bunch of interesting possibilities:
* Have a function to automatically perform the regex replacement and
present the result to you as a string.
* Or if you want to be even lazier, have a NAPTR function that will take a
NAPTR result and convert that into an ast_dns_query that you can then resolve
yourself.
* Or if you want to be even lazier, have an async NAPTR function not return
results until it boils down to an A or AAAA record.
* NAPTR and SRV have the potential for functions that allow for you to iterate
over the different priority records that are returned.
* NAPTR and SRV could have fallback functions built into them, too.
As far as the examples are concerned, they're good, but I feel like an example
that uses NAPTR (which subsequently results in an SRV lookup) could be useful.
You may find that when you write the example, manual construction of further
queries based on the data that you are retrieving from NAPTR records leads you
to want to implement some of the suggested high-level functions for it.
Regarding your question about DNS being core or module-based, it kind of
depends on a few factors:
* What code in Asterisk is going to be updated to use the new DNS API? If older
code is not going to be updated to use the new DNS API, then I think that
resolvers should live in loadable modules. If someone upgrades to Asterisk 15
and plans to continue using chan_sip and hasn't had issues with DNS, then they
probably aren't going to be too thrilled about having to download c-ares or
libunbound, only to not actually use it for anything. If old code is going to
be updated to use the new DNS API, then...
* Are we planning on keeping any of the old DNS code from Asterisk around as a
resolver implementation choice? If so, then again, I suggest having resolvers
be separate modules. This way we still provide an upgrade path that does not
have new dependencies or new underlying implementations.
My personal opinion is that all old code should be updated to use the new DNS
API (without actually changing behavior, even if the old behavior is incorrect)
and that the old implementation should be thrown away. If we go that route,
then resolvers should be part of the core since DNS resolution is a fundamental
thing in most Asterisk installations.
There is one further thing I can think of here, and that is the current dnsmgr
code in Asterisk. Is that simply going to be updated to use the new DNS API, or
is dnsmgr going to get some sort of overhaul to provide new functionality that
the underlying DNS API makes it capable of?
- Mark Michelson
On Feb. 23, 2015, 12:25 a.m., Joshua Colp wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4437/
> -----------------------------------------------------------
>
> (Updated Feb. 23, 2015, 12:25 a.m.)
>
>
> Review request for Asterisk Developers.
>
>
> Repository: Asterisk
>
>
> Description
> -------
>
> A wiki page is present at:
>
> https://wiki.asterisk.org/wiki/display/~jcolp/Asterisk+DNS+API
>
> Which details a new DNS API for Asterisk. This exists as a thin wrapper over
> other resolver libraries. The core part provides a common interface and some
> additional features, such as NAPTR/SRV parsing and recurring lookups.
> Examples are provided which cover the common use cases for the API.
>
> Some stuff to think about:
>
> 1. Does this encompass everything we think a low level API should?
> 2. Are there any higher level APIs that would be useful to have?
> 3. Is the usage intuitive and easy?
> 4. Are there other examples which would help?
> 5. Do we want resolvers to be actual modules or keep them in-core?
> 6. Anything else you think of
>
> Have at it!
>
>
> Diffs
> -----
>
>
> Diff: https://reviewboard.asterisk.org/r/4437/diff/
>
>
> Testing
> -------
>
> I've logically run through the API and examples to ensure they provide what
> is needed for the future, to make them as easy as possible to use, and to
> ensure higher level APIs can be created.
>
>
> Thanks,
>
> Joshua Colp
>
>
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
