Opened 9 years ago

Last modified 4 years ago

#3324 defect new

Write an SMTP server tutorial

Reported by: Jonathan Lange Owned by: Stacey Sern
Priority: low Milestone:
Component: mail Keywords: documentation
Cc: Thijs Triemstra Branch: branches/smtp-server-tutorial-3324-2
branch-diff, diff-cov, branch-cov, buildbot
Author: argonemyth, shira

Description (last modified by Thijs Triemstra)

There's no server tutorial HTML file in trunk or in http://twistedmatrix.com/documents/current/mail/tutorial

It would be good to have such a tutorial.

Change History (19)

comment:1 Changed 9 years ago by Jean-Paul Calderone

Owner: changed from Jean-Paul Calderone to Jonathan Lange

That's because I never wrote it. I don't know what to do with this ticket.

comment:2 Changed 9 years ago by Jonathan Lange

Description: modified (diff)
Priority: normallow
Summary: SMTP server tutorial missingWrite an SMTP server tutorial

comment:3 Changed 7 years ago by <automation>

Owner: Jonathan Lange deleted

comment:4 Changed 6 years ago by Thijs Triemstra

Cc: Thijs Triemstra added
Description: modified (diff)

comment:5 Changed 5 years ago by argonemyth

Owner: set to argonemyth

comment:6 Changed 5 years ago by argonemyth

Author: argonemyth
Branch: branches/smtp-server-tutorial-3324

(In [35314]) Branching to 'smtp-server-tutorial-3324'

comment:7 Changed 5 years ago by argonemyth

Keywords: review added
Owner: argonemyth deleted

A SMTP tutorial is added, please review smtpserver.xhtml, thanks!

comment:8 Changed 5 years ago by Thijs Triemstra

Keywords: review removed
Owner: set to argonemyth

Thanks for working on more documentation!

  1. the tutorial is missing from the index at doc/mail/index.xhtml
  2. I would rename smtpserver/smtpserver.xhtml to smtpserver/index.xhtml
  3. In 'SMTP Server 1':
    1. The reference to <code class="API">twisted.mail.smtp</code> is broken and should not be broken into 2 separate lines
    2. The word module is missing behind This tutorial will show you how to use the twisted.mail.smtp
    3. In which save emails sent the word save should be saves
    4. In It's not even listening at any port. the word at should be on
  4. In 'SMTP Server 2':
    1. The last print of the code example suggests the server quits with a SIGINT after that TypeError is encountered when in fact it's only shutdown when issuing ^C. I would add this and perhaps split it into two.
  5. In 'SMTP Server 4':
    1. Imports in smtpserver-4.tac (and all other related .tac files) should go to the top of the file
    2. Lines start in (Lines start with should be lines starting
    3. lines start with client are the stuff we type in should be lines starting with client are the commands we enter at the telnet prompt
  6. In 'SMTP Server 5':
    1. In order to have the server to 'send' messages, should be To make the server 'send' messages,
    2. TutorialESMTP should be wrapped in <code>
    3. The word The is missing before validateTo method returns
    4. The word the is missing before validateFrom in we simply made validateFrom method
    5. The example isn't working for me it seems, I get the following output:
      $ telnet localhost 2025
      Trying 127.0.0.1...
      Connected to localhost.
      Escape character is '^]'.
      220 bubba NO UCE NO UBE NO RELAY PROBES ESMTP
      EHLO localhost
      250 bubba Hello 127.0.0.1, nice to meet you
      MAIL FROM: <tom@example.com>
      250 Sender address accepted
      RCPT to: <alice@argonemyth.com>
      250 Recipient address accepted
      DATA
      354 Continue
      From: tom@example.com
      To: alice@argonemyth.com
      Subject: Test message to Alice
      
      This is a test message.
      .
      250 Delivery in progress
      From: alice@example.com
      500 Command not implemented
      

That's it for now, I'll complete the review after the next round (and when that example is working).

comment:9 in reply to:  8 Changed 5 years ago by Thijs Triemstra

Replying to thijs:

  1. the tutorial is missing from the index at doc/mail/index.xhtml
  1. doc/mail/tutorials/ needs an index.xhtml that needs to be linked from the root doc/index.xhtml. this also needs to be linked from doc/mail/index.xhtml

comment:10 Changed 5 years ago by argonemyth

Keywords: review added
Owner: argonemyth deleted

Hey thijs,

Thanks for the detailed review! I modified branch 'smtp-server-tutorial-3324' according to your comments. Please continue the review when you have some time.

comment:11 in reply to:  10 Changed 5 years ago by Thijs Triemstra

Keywords: review removed
Owner: set to argonemyth

Replying to argonemyth:

Hey thijs,

Thanks for the detailed review! I modified branch 'smtp-server-tutorial-3324' according to your comments. Please continue the review when you have some time.

Thanks for the update.

  1. In SMTP Server 1:
    1. Instead of 'We can shutdown it by issuing' use 'We can shut it down by issuing'
    2. It might not be clear to everyone what ^C means, perhaps you can add behind that, something like '(Ctrl-C)'? But there's no need to add this to other occurrences later on as well.
  2. In SMTP Server 2:
    1. <code>internet.TCPServer</code> should be an API link instead
  3. In SMTP Server 3:
    1. <code>ServerFactory.buildProtocol</code> should be an API link
  4. In SMTP Server 5:
    1. <code>IMessage</code> should be an API link
    2. ' we will do a little bit code refactoring' should be ' we will do a little bit of code refactoring'
  5. In SMTP Server 7:
    1. <code>smtp.IMessageDelivery</code> should be an API link
    2. the word 'the' is missing in 'in <code>TutorialESMTPFactory</code> class:'
    3. <code>TutorialDelivery on line 424 should not be split on 2 lines
  6. In SMTP Server 8:
    1. <code>smtp.IMessageDelivery</code> should be an API link
    2. smtp.IMessageDeliveryFactory</code> should be an API link
    3. the link on line 459 breaks when split across 2 lines like that
    4. Get rid of 'I' in 'I hope you appreciate', maybe use 'Hopefully' instead
    5. 'touch' should be 'touching' on line 492 and the word 'the' is missing in front of 'ESMTP protocol'
  7. In Conclusion:
    1. Rewrite the reference to 'I'
    2. IMessageDeliveryFor on line 501 is broken
    3. On the same line, 'interface...To' needs a space
    4. Get rid of the 'To me,' reference
    5. <code>Twisted</code> should be just Twisted on line 502
    6. replace 'weird' with 'special' on line 503

Also run this against buildbot when you put back up for review.

comment:12 Changed 4 years ago by Stacey Sern

Owner: changed from argonemyth to Stacey Sern

comment:13 Changed 4 years ago by Stacey Sern

(In [39211]) Revised tutorial. refs #3324

comment:14 Changed 4 years ago by Stacey Sern

The revisions to the tutorial go beyond the comments in #11. Examples #5-8 have been changed so that the message is written to stdout instead of a file. It was much clearer that way to see how messages were being captured (in one of the old examples, one message was writing over another with the same file name). Also, example #8 was completely rewritten to better reflect the usage of IMessageDeliveryFactory. There were a lot of other minor changes to hopefully improve clarity and grammar.

Since so much of the file was being changed anyway, I modified the file so that every sentence is one continuous line.

comment:15 Changed 4 years ago by Stacey Sern

Author: argonemythargonemyth, shira
Branch: branches/smtp-server-tutorial-3324branches/smtp-server-tutorial-3324-2

(In [39227]) Branching to 'smtp-server-tutorial-3324-2'

comment:16 Changed 4 years ago by Stacey Sern

(In [39228]) Merge forward (no conflicts) refs #3324

comment:17 Changed 4 years ago by Stacey Sern

Keywords: review added
Owner: Stacey Sern deleted

comment:18 Changed 4 years ago by Richard Wall

Keywords: review removed
Owner: set to Stacey Sern

Thanks shira,

This new howto is extremely useful.

It made complete sense until section8, where I got confused.

Judging by the diff it looks like all the tac files already existed. Is that right? Seems strange that they were never linked up.

Notes:

Points:

  1. "by issuing Control-C" > "By typing Control-C"
  2. "using the twistd command line" missing end colon
  3. Typo "continues to listens"
  4. "issuing C"
  5. server3
    1. Maybe add a note about how to quit telnet or just mention that the telnet client will quit when the server is stopped.
      [richard@zorin ~]$ telnet localhost 2025
      Trying 127.0.0.1...
      Connected to localhost.
      Escape character is '^]'.
      ^]
      
      telnet> quit
      Connection closed.
      
  6. server5
    1. Use zope.interface.implementer decorator which is better practice than the old implements
    2. "prepended to the message" -- should that be "appended"?
    3. The paragraph beginning "If you check the base class implementation of" is interesting, but it got quite technical and a little bit difficult to read. Maybe put it in a <div class="note"> box.
  7. server6
    1. "replace that code with the creation of a new subclass" > "replace that code with a new subclass"
    2. I wasn't convinced that this refactoring was worth an extra step...but it's ok.
  8. server7
    1. It seems strange that delivery can't be passed as an ESMTP constructor argument
    2. ...and in fact the SMTP constructor does accept a delivery and a deliveryFactory argument.
    3. So why has this been hidden in ESMTP?
    4. I wonder if it's because they wanted to discourage the overriding of delivery?
    5. Maybe not, but if delivery is meant to be used, then it needs to be better documented in the API docs.
    6. Raise a ticket for that if you agree.
    7. And raise a ticket for allowing passing a deliveryFactory to ESMTP.init if you agree?
  9. server8
    1. This is getting complicated.
    2. I'm thinking that people will be reading this tutorial for two main reasons:
      1. to implement a custom SMTP endpoint server (is that the correct term?) - ie other SMTP servers are delivering messages to me because I'm listening on the IP address of the MX record host and I'll deliver the message to a users post box (maybe a database)
  1. to implement a custom "smart host" SMTP server which receives connections from SMTP mail clients (eg outlook), looks up MX records for recipients, and dispatches the separate messages to the SMTP server for each recipient. (maybe with some custom filtering or archiving system).
  1. So why would I ever want to use the NullMessage class?
  1. Why doesn't ESMTP allow me to return None from validateTo -- to indicate that I don't want to collect the DATA?
  1. I'm probably just not getting it. But it seems to me that the ESTMP API is convoluted and makes the tutorial more complex than it needs to be.
  1. Conclusion
    1. I think you should include some ideas for next steps
    2. Give some real world examples of why you might want to write a custom ESTMP server.
    3. Give some links to other examples of twisted.mail servers if there are any.
    4. Maybe add some links to SMTP and ESMTP RFCs
  2. I think the tac files should at least adhere to the Twisted

whitespace standard (eg 3 lines between classes)

  1. In wiki:Plan/BetterExamples we've even talked about requiring docstrings and epydoc in the example scripts. What do you think about that?

Thanks again. Please resubmit for review after answering or addressing the points above.

-RichardW.

comment:19 Changed 4 years ago by hawkowl

Just a quick note that the existing emailserver.tac example has p.delivery = self.delivery which completely negates the authentication checking.

Note: See TracTickets for help on using tickets.