Skip to content

smtpd-as-a-script feature should be documented and should use argparse #55469

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
masklinn mannequin opened this issue Feb 20, 2011 · 17 comments
Closed

smtpd-as-a-script feature should be documented and should use argparse #55469

masklinn mannequin opened this issue Feb 20, 2011 · 17 comments
Labels
docs Documentation in the Doc dir stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@masklinn
Copy link
Mannequin

masklinn mannequin commented Feb 20, 2011

BPO 11260
Nosy @warsaw, @rhettinger, @ncoghlan, @merwok, @bitdancer, @masklinn, @berkerpeksag
Files
  • smtpd-to-argparse.diff: Port smtpd to argparse
  • smtpd-as-script-doc.diff: Documentation of smtpd-as-a-script
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2018-01-10.00:54:36.213>
    created_at = <Date 2011-02-20.20:03:50.521>
    labels = ['type-feature', 'library', 'docs']
    title = 'smtpd-as-a-script feature should be documented and should use argparse'
    updated_at = <Date 2018-01-10.00:54:36.212>
    user = 'https://github.com/masklinn'

    bugs.python.org fields:

    activity = <Date 2018-01-10.00:54:36.212>
    actor = 'barry'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-01-10.00:54:36.213>
    closer = 'barry'
    components = ['Documentation', 'Library (Lib)']
    creation = <Date 2011-02-20.20:03:50.521>
    creator = 'xmorel'
    dependencies = []
    files = ['20812', '20813']
    hgrepos = []
    issue_num = 11260
    keywords = ['patch']
    message_count = 17.0
    messages = ['128917', '128918', '128998', '129042', '129044', '129047', '129048', '129078', '129117', '129123', '129132', '129157', '129467', '140278', '161834', '223270', '309750']
    nosy_count = 9.0
    nosy_names = ['barry', 'rhettinger', 'ncoghlan', 'eric.araujo', 'r.david.murray', 'xmorel', 'docs@python', 'BreamoreBoy', 'berker.peksag']
    pr_nums = []
    priority = 'low'
    resolution = 'wont fix'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue11260'
    versions = ['Python 3.5']

    @masklinn
    Copy link
    Mannequin Author

    masklinn mannequin commented Feb 20, 2011

    argparse has been merged to the standard library in 3.2, and (tell me if I'm wrong) would therefore be the "best-practices" way to parse command-line arguments.

    Numerous stdlib modules can be used as scripts, but they tend to have ad-hoc documentation (if they are at all documented) and arguments parsing (using any of the 4 available methods in Python: straight from sys.argv, getopt, optparse and argparse).

    I picked smtpd as a first shot since it does something useful (SMTP proxy) and has a pretty good (if ad-hoc) command-line documentation.

    smtpd is currently using getopt for its options parsing and the argument parsing is very cleanly factored: the port only had to replace the implementation of the parseargs function (and add and remove some helpers).

    • The port keeps the existing arguments semantics (including the mandatory host:port syntax for the local and remote specs if overridden)

    • The port tries to maintain the old error messages, but due to the way argparse works (or the way I used it for the specs) the parity is not perfect when providing incorrect specs

    • The CLI help uses argparse's formatting, and the documentation for the local and remote specs was set on these arguments rather than as an epilog. The version string was also removed from the help screen

    • Because they are set by argparse's arguments validation, the status codes on incorrect arguments are slightly different:

      • running smtpd.py as a regular user without the --nosetuid flag still exits with status 1
      • providing incorrect spec formats (missing or non-int port) or providing too many positional arguments (3 or more) now exits with status 2 (formerly 1)

    @masklinn masklinn mannequin assigned docspython Feb 20, 2011
    @masklinn masklinn mannequin added docs Documentation in the Doc dir stdlib Python modules in the Lib dir labels Feb 20, 2011
    @masklinn
    Copy link
    Mannequin Author

    masklinn mannequin commented Feb 20, 2011

    Second patch: documenting smtpd-as-a-script in the module's rst

    @bitdancer
    Copy link
    Member

    Thanks for taking an interest in this and working up a patch. Unfortunately I do have some concerns.

    As far as I know the only module that currently uses argparse is compileall, which was only changed recently.

    Do tests currently exist for smtpd run as a script? If not, our experience with converting compileall to argparse indicates a thorough test suite is needed (and even so we missed some things we hadn't thought to test).

    We converted compileall because its help output was broken, but in retrospect it might have been better to fix the help output in the existing code. In other words, if the current code works, is "updating" it a sufficient reason to change it, considering the chances of introducing new bugs? The answer might be yes, but I don't think it is obviously yes.

    @bitdancer bitdancer added the type-feature A feature request or enhancement label Feb 21, 2011
    @masklinn
    Copy link
    Mannequin Author

    masklinn mannequin commented Feb 22, 2011

    Do tests currently exist for smtpd run as a script?

    I have to confess I didn't think to check.

    If not, our experience with converting compileall to argparse indicates a thorough test suite is needed (and even so we missed some things we hadn't thought to test).

    OK, so if there is no test suite currently I should create one, and if there is one I should ensure it's complete? I guess I should use compileall as an example of how to test modules-as-scripts if the former? Overall, smtpd-as-a-script is really pretty simple, it totals 28 lines apart from the argument parsing itself (which is a bit under 60 lines ignoring the help text/pattern and gets a bit under 50 including it post-patch), and as I mentioned the only part which actually needed changing is the arguments parsing, which was very well factored out.

    In other words, if the current code works, is "updating" it a sufficient reason to change it, considering the chances of introducing new bugs?

    I'm not sure, but one of the ways I see standard libraries is not just as "ready to use" code, it's also as a repository of how to do things. And to that end, if argparse is now considered the canonical way to parse command-line arguments (which I would expect), there are very few examples of how to do things in the stdlib itself (though there are examples outside of it, due to the life argparse had pre-stdlib).

    It also rose to the occasion as I was wondering about the numerous standard library modules-as-scripts which are either undocumented or under-documented: because it had a good command-line documentation and a clean separation between the configuration (options parsing) and the actual execution, but no module documentation (in Doc/) it seemed like a good starting point: if it's not feasible to correctly convert "best cases" (and smtpd is probably one, short of modules using optparse probably) then the whole idea is stillborn: I do not see how it would be possible to fare better on some of the fully undocumented modules using manual options parsing, yet it would have to be expected.

    @rhettinger
    Copy link
    Contributor

    Xavier, I think these efforts are misguided in several ways:

    • Many of the undocumented command-line interfaces are
      intentionally undocumented -- they were there for the
      convenience of the developer for exercising the module
      as it was being developed and are not part of the official API.
      Most are not production quality and would have been done
      much differently if that had been the intent.

    • The standard library's primary purpose is to be a library
      for use in user programs, not to serve as a suite of applications.
      We do have a few applications such as pydoc that assist
      development, but that is not what we want to do for most
      modules. Real apps need more features than we usually
      offer and they need much faster development cycles than
      supported by our 18-24 month release cycle.

    • To the extent the standard library serves as a set of
      examples, that is just side benefit. There are other
      ways to make code specifically for examples (we include
      tons of examples in the documentation; we had a Demos
      directory; there is the wiki; and there is the ASPN
      Cookbook site; etc)

    • David Murray is correct is pointing-out that converting
      to argparse risks changing the semantics and possibly
      breaking someone's existing uses of the current interface.

    • Though argparse is the shiny new kid on the block and
      it should be used for new code, there is no imperative
      to go change all existing command-line scripts. We haven't
      even done that same exercise for the new string formatting.

    • All that being said, there are some exceptions and it
      make may sense to document the interface in some where
      we really do want a command-line app. I'll look at
      any patches you want to submit, but try to not go wild
      turning the library into a suite of applications. For
      the most part, that is not what the standard library
      is about.

    @rhettinger rhettinger assigned rhettinger and unassigned docspython Feb 22, 2011
    @masklinn
    Copy link
    Mannequin Author

    masklinn mannequin commented Feb 22, 2011

    any of the undocumented command-line interfaces are intentionally undocumented -- they were there for the convenience of the developer for exercising the module as it was being developed and are not part of the official API.

    I can understand that, but it's not clear from the source code which is which, and for several of them third-parties have taken up the convenience. Maybe a more formal process of defining which tools are to be considered "official" and which are easter-eggs is needed? In any case, even for "easter eggs" surely a good command-line documentation is a good idea (even if the module documentation side is not added due to the "easter egg" status) isn't it?

    The standard library's primary purpose is to be a library for use in user programs, not to serve as a suite of applications.

    Sure, and I don't think most of the module-as-scripts have what it takes to be seen as applications, but many are useful and/or interesting helpers/shortcuts for day to day tasks. http.server and smtpd are good examples of useful modules-as-scripts (http.server being the exact opposite of smtpd in that it has a module-as-script documentation but no documentation whatsoever on the command-line)

    Real apps need more features than we usually offer and they need much faster development cycles than supported by our 18-24 month release cycle.

    I think my suggestion has been misunderstood: I don't want to turn these into real apps, they're fine as basic scripts/helpers to spend 5mn on a task instead of half an hour. I think they deserve better than to be documented and known through StackOverflow or Reddit "what are the -m stdlib features" lists.

    To the extent the standard library serves as a set of examples, that is just side benefit. There are other ways to make code specifically for examples (we include tons of examples in the documentation; we had a Demos directory; there is the wiki; and there is the ASPN Cookbook site; etc)

    Sure, but I think it's still an important driver of how things "are done" in that they're *concrete* examples, not abstract (of course the Cookbook is generally concrete as well). I'm not discounting the importance or quality of the rest of the documentation at all, or at least that was not my intention.

    All that being said, there are some exceptions and it make may sense to document the interface in some where we really do want a command-line app. I'll look at any patches you want to submit, but try to not go wild turning the library into a suite of applications. For the most part, that is not what the standard library is about.

    As I said, my only intention here is be to document (and argparsify/formalize) what is already there. I considered doing more (e.g. for the specific case of smtpd-as-a-script making ports optional even when hosts are specified, that kind of things) but decided against this distraction, I really just want to make existing module-as-script features simpler to discover and use.

    That said, do you have guidelines of which areas this idea would be most interestingly/efficiently applied? Maybe a list of modules-as-scripts you know are used regularly and could benefit from some improvements in their interface or documentation?

    @masklinn
    Copy link
    Mannequin Author

    masklinn mannequin commented Feb 22, 2011

    One more note I forgot previously: the conversion of as much scripts as possible to argparse would be for three reasons:

    • Make behavior consistent across the board (e.g. -h/--help)
    • Make CLI documentation format consistent across the board either so users know what to expect and when
    • Provide easy to reach (for users) examples of using argparse

    @warsaw
    Copy link
    Member

    warsaw commented Feb 22, 2011

    I'd support updating to argparse *if* you write a test suite for full coverage of the existing cli first. Once that passes, you'll have more confidence in your port. Modernizing the argument parsing in that case would be a useful addition. Not critical, but on the whole I think a good thing.

    @masklinn
    Copy link
    Mannequin Author

    masklinn mannequin commented Feb 22, 2011

    Barry, do I correctly understand your comment to mean I should write end-to-end tests of the CLI (until reaching the already tested "meat" of smtpd), not just the CLI options parsing?

    @warsaw
    Copy link
    Member

    warsaw commented Feb 22, 2011

    On Feb 22, 2011, at 08:15 PM, Xavier Morel wrote:

    Barry, do I correctly understand your comment to mean I should write
    end-to-end tests of the CLI (until reaching the already tested "meat" of
    smtpd), not just the CLI options parsing?

    Given the way the __main__ is written, that's probably more thorough. Testing
    just the parsing alone would be uninteresting I think.

    @rhettinger
    Copy link
    Contributor

    As I said, my only intention here is be to document
    (and argparsify/formalize) what is already there.

    In a handful of cases, that would be useful; however,
    for the most part, these APIs were undocumented for
    a reason. Some of the command-line interfaces were
    slapped together without much of a design effort; some
    were little more than quick-and-dirty ad-hoc tests.
    It would be a mistake to "document and formalize"
    accidental mini-apps.

    There's no harm in adding --help or a usage example,
    but please avoid making behavior guarantees that we
    really don't want to have to live with.

    Only "document and formalize" the parts that are well
    thought out.

    @masklinn
    Copy link
    Mannequin Author

    masklinn mannequin commented Feb 23, 2011

    Only "document and formalize" the parts that are well thought out.

    I don't believe I have the knowledge, right or ability to make that call for any module or package but a pair of extremely obvious ones (http.server seems a pretty good candidate as it's documented in the module doc, and is just missing a CLI help). Should I create a bug and nosy the people who have been involved in the module to get their opinion for each one? (as with this one, but maybe with a different wording/initial proposal) For this one, David expresses concerns on the stability on the interface (and the point of the idea), you express even bigger concerns with the idea itself and more generally modules-as-scripts in the stdlib (not their existence so much as their official support, which full documentation would imply, if I didn't misunderstand you), and Barry seems to okay the idea as long as an extensive test suite is created beforehand to ensure there is no regression, is that sufficient to go ahead with more work and defer the final decision for when that will be done?

    Also, if this is to become an actual project if the smtpd stuff ever reaches fruition (though not necessarily a big or impactful one), should there be a meta-bug? (I know they're used in many bugzilla projects and I see the tracker handles dependencies)

    There's no harm in adding --help or a usage example, but please avoid making behavior guarantees that we really don't want to have to live with.

    I'm not sure what that results in: for "quick hacks" which are not to be officially supported, does this mean fixing the CLI (adding a help for tools missing one) is OK but no more? Is an argparse switch still acceptable? Tests for the CLI tool? I'm guessing documentation in the module would definitely be off limits for those, is my interpretation correct? And again, who would be allowed to make the call on which modules-as-scripts may be considered supported, and can't be?

    With this one, I created separate patches for the documentation and the CLI parsing alterations, which would allow merging the CLI without adding it to the official documentation (which would I think imply a lack of official support), would that be OK for future works, if this one pans out?

    @merwok
    Copy link
    Member

    merwok commented Feb 25, 2011

    Meta-bugs are useful for bookkeeping purposes like the implementation of a PEP or a broad feature like “bytes/text model in email” <wink at David>. Just open one report for each module you want to add doc for.

    @rhettinger rhettinger removed their assignment Mar 22, 2011
    @merwok
    Copy link
    Member

    merwok commented Jul 13, 2011

    Here are comments on the doc patch.

    +Run as a script
    +---------------
    I’d say “Command-Line Interface”.

    +``smtpd`` is a pluggable RFC 2821-compliant SMTP proxy.
    :mod:`smtpd` is also a pluggable etc.

    +.. program:: smtpd.py
    Strip the .py

    + the ``setuid``
    Please use markup like :func:`os.setuid`.

    + flag in order to run ``smtpd`` as a regular user.
    Use :mod:`smtpd` or :program:`smtp` (not very important).

    + Turns on verbose debugging prints (to stderr)
    Turn on verbose debbugging, which prints to stderr.

    • The concrete SMTP proxy class smtpd should use to perform its
    • proxying.
      Could you add a link to a section of the doc that defines such classes?

    +.. option:: localhost:localport
    Currently undocumented.

    @ncoghlan
    Copy link
    Contributor

    I created bpo-14945 as a suggestion to add a simple page to http://docs.python.org/dev/using/index.html that will provide a central reference to the module documentation for modules with officially supported behaviour when used with the "-m" switch.

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Jul 16, 2014

    Where do we stand with this and bpo-14945 ?

    @warsaw
    Copy link
    Member

    warsaw commented Jan 10, 2018

    I'm closing this as won't fix since smtpd.py is deprecated and will likely not get any future development. Please see aiosmtpd as a much better third party replacement.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    docs Documentation in the Doc dir stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants