Skip to content
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

Better documentation and example #175

Open
davidhcefx opened this issue Jan 10, 2023 · 0 comments
Open

Better documentation and example #175

davidhcefx opened this issue Jan 10, 2023 · 0 comments

Comments

@davidhcefx
Copy link

davidhcefx commented Jan 10, 2023

Description

This package is really a simple and working client/server for RADIUS. However, I found that the documentation is a bit too simple, and have already encountered some issues when adopting pyrad.

  1. The first (and only) example in the doc is the Introduction section. The first issue I encountered was that there is no description of what Dictionary("dictionary") really is. (Is it a path to a file, or is it a special keyword?) Under pyrad.client there's also no description for the argument dict, unless delving into the code to figuring out that:

    • :param dict: RADIUS dictionary under Client.__init__, as well as
    • :param dict: path of dictionary file or file-like object to read under Dictionary.__init__.
    • [Suggestion]: Copy the inline doc for each constructors to the main doc.
  2. After that, I started with creating an auth packet, but quickly got confused by what are the valid arguments I could pass to CreateAuthPacket? The pyrad.client page also described nothing for **args. Another inconsistency is that attributes were been passed to function with underscores (eg. User_Name), but were been accessed with hyphens (eg. req["User-Password"]). It turns out that underscores are for args and hyphens are for direct access, which is another undocumented thing.

    • [Suggestion]: Add a sentence to the index page: "For setting attributes to Client object, you need to replace underscores with hyphens."
    • [Suggestion]: Add some description for the valid arguments that could be passed to CreateAuthPacket.
  3. After successfully sending auth requests to the server. I then try to craft a accounting request. However, I found that there's one more thing that's misleading! The example passed code=pyrad.packet.AccessRequest to CreateAuthPacket, but by delving into the code, I found that this is in fact absolutely unnecessary, because the AuthPacket constructor already set code=AccessRequest by default!

    • [Suggestion]: Remove the misleading argument code=pyrad.packet.AccessRequest from the example, or choose to document that it is not necessary to pass that argument.

Other Suggestions

  • Apart from the above suggestions, it would be better to:
    • Provide another example for doing an accounting request:
      from __future__ import print_function
      from pyrad.client import Client
      from pyrad.dictionary import Dictionary
      
      srv = Client(server="localhost", secret=b"Kah3choteereethiejeimaeziecumi",
                   dict=Dictionary("dictionary"))
      
      # create request
      req = srv.CreateAcctPacket(User_Name="wichert",
                                 NAS_Identifier="localhost", Acct_Status_Type=1)
      
      # send request
      reply = srv.SendPacket(req)
@davidhcefx davidhcefx changed the title Better example and documentation Better documentation and example Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants