Opened 11 months ago

Closed 9 months ago

#6864 enhancement closed fixed (fixed)

Add narrative documentation showing how to use twisted.names to create a custom server

Reported by: rwall Owned by: rwall
Priority: normal Milestone:
Component: names Keywords: documentation
Cc: Branch: branches/custom-server-documentation-6864-2
(diff, github, buildbot, log)
Author: rwall Launchpad Bug:

Description

The twisted.names narrative documentation shows how to load an authoritative server from a pyzone file and twistd.

But it doesn't demonstrate how t.names.authority can be used to create a custom authoritative server.

Nor does it show how authoritative server can be combined with other resolvers and how certain results can be overridden.

hynek showed me (rwall) some interesting example code at the Bristol sprint which would be a good starting point for this ticket.

Attachments (1)

auth-dns-docs-6864.patch (11.3 KB) - added by rwall 9 months ago.
An example of a custom DNS resolver with pass through

Download all attachments as: .zip

Change History (12)

Changed 9 months ago by rwall

An example of a custom DNS resolver with pass through

comment:1 Changed 9 months ago by rwall

  • Owner set to rwall
  • Status changed from new to assigned
  • Summary changed from Add narrative documentation showing how to use twisted.names.authority to create a custom authoritative server to Add narrative documentation showing how to use twisted.names to create a custom server

comment:2 Changed 9 months ago by rwall

  • Author set to rwall
  • Branch set to branches/custom-server-documentation-6864

(In [41236]) Branching to 'custom-server-documentation-6864'

comment:3 Changed 9 months ago by rwall

  • Branch changed from branches/custom-server-documentation-6864 to branches/custom-server-documentation-6864-2

(In [41272]) Branching to 'custom-server-documentation-6864-2'

comment:4 Changed 9 months ago by rwall

  • Keywords review added
  • Owner rwall deleted
  • Status changed from assigned to new

Ready for review in log:branches/custom-server-documentation-6864-2

  • Added some examples of custom (non-authoritative) DNS servers
  • And some brief narrative documentation explaining how they work

Over to you hynek :)

If you can describe the other case that you showed me in Bristol I might try and describe that too....here or in another ticket.

comment:5 Changed 9 months ago by khorn

  • Owner set to khorn

comment:6 follow-up: Changed 9 months ago by khorn

  • Keywords review removed
  • Owner changed from khorn to rwall

Review Notes:

  • I don't see simple_server.py or override_server.py. Did you forget to check them in, or am I just blind?
  • Note that in RestructuredText when marking up a literal block (using ::), you can put the :: at the end of the preceding line, similar to how one might use a colon to lead into the block. So instead of:
    here's some code
    ::
    
        code goes here
    

you'd have something like:

here's some code::

    code goes here

I think this makes the markup look a bit nicer, and would recommend changing to this style, but don't block the ticket on this if you disagree.

  • Maybe I'm being a bit overly-pedantic, but in yoru explanatory note section, you say:
    It takes a list of :api:`twisted.internet.interfaces.IResolver <IResolver>` providers and queries each one in turn until it receives an answer.
    

Perhaps it should be more exact, like:

It takes a list of :api:`twisted.internet.interfaces.IResolver <IResolver>` providers and queries each one in turn until either it receives an answer, or they all fail.

Probably you can make that sentence flow a little better, but I think it should be made clear that it isn't going to repeat the chain over and over. Maybe this change isn't strictly necessary, but I think it makes things a little more clear for those who aren't very familiar with Twisted.

The narrative itself looks pretty good, but it's difficult to say without seeing the actual code examples. Please add the code or point out what I'm missing and resubmit for review.

comment:7 Changed 9 months ago by khorn

  • Keywords review added
  • Owner changed from rwall to khorn

comment:8 in reply to: ↑ 6 Changed 9 months ago by rwall

Thanks for the review Kevin.

My comments and answers are below...

Replying to khorn:

Review Notes:

  • I don't see simple_server.py or override_server.py. Did you forget to check them in, or am I just blind?

They are there, but may not be shown in the Trac diff because they're new files.
Trac does show the links to those files at the top of the diff page though (in green).

  • Note that in RestructuredText when marking up a literal block (using ::), you can put the :: at the end of the preceding line, similar to how one might use a colon to lead into the block. So instead of:

Yeah, I had tried that, but I was confused because it renedered as a single : at
the end of the line, or sometimes a :: if there was a preceding full stop.

And neither form resulted in syntax highlighting, so I've switched to the
'.. code-block:: console' syntax instead.

Probably you can make that sentence flow a little better, but I think it should be made clear that it isn't going to repeat the chain over and over. Maybe this change isn't strictly necessary, but I think it makes things a little more clear for those who aren't very familiar with Twisted.

I agree. I've changed it. And added a little more detail about the use of DomainError.

Ready for another review.

Thanks again.

comment:9 Changed 9 months ago by khorn

  • Keywords review removed

comment:10 Changed 9 months ago by khorn

  • Owner changed from khorn to rwall

This looks great and could be merged as it is, but here's a couple of things which might improve it:

  • Some of the lines in the RestructuredText files are pretty long, and should probably be shortened
  • In the override_server.py example, in DynamicResolver._doDynamicResponse the return values are a bit opaque. I'm not too concerned about this, since it is explained in the narrative text, but you might consider adding each of the three return values to temporary/throwaway variables with appropriate names, and then returning those names, just to help clarify things.

Other than that, looks great. Merge when ready.

comment:11 Changed 9 months ago by rwall

  • Resolution set to fixed
  • Status changed from new to closed

(In [41335]) Merge custom-server-documentation-6864-2

Author: rwall
Reviewer: khorn
Fixes: #6864

Added some narrative documentation showing how to create a custom DNS
server. Also added a few introductory paragraphs to the twisted.names
documentation index page.

Note: See TracTickets for help on using tickets.