Skip to content

Move $nameserver from ExecutorInterface to UdpTransportExecutor - #135

Merged
jsor merged 1 commit into
reactphp:masterfrom
clue-labs:nameserver-interface
Jul 11, 2019
Merged

Move $nameserver from ExecutorInterface to UdpTransportExecutor#135
jsor merged 1 commit into
reactphp:masterfrom
clue-labs:nameserver-interface

Conversation

@clue

@clueclue commented Jul 11, 2019

Copy link
Copy Markdown
Member

This changeset removes the $nameserver argument from the ExecutorInterface and adds it as a constructor attribute to the UdpTransportExecutor. This allows for a significant clean up in our APIs as most of the existing implementations of ExecutorInterface do not care about the nameserver and simply pass it through to some underlying executor. This also allows for future optimizations (such as reusing a single socket for multiple lookups) and helps with future TCP/IP executors (#19) and also multiple DNS servers by implementing fallback strategies as a hierarchy of executors (#6). These APIs are somewhat internal and it's unlikely this will affect many consumers of this package.

// old$executor = newUdpTransportExecutor($loop);
$promise = $executor->query($nameserver, $query);
// new$executor = newUdpTransportExecutor($nameserver, $loop);
$promise = $executor->query($query);

Additionally, we can now validate the nameserver address and throw an InvalidArgumentException if it is invalid.

Refs #131
Refs #128

@jsor
jsor merged commit 9f5bf59 into reactphp:masterJul 11, 2019
@clue
clue deleted the nameserver-interface branch July 11, 2019 12:46
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@clue@jsor@WyriHaximus