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

Add socketserver running property #58583

Open
giampaolo opened this issue Mar 20, 2012 · 4 comments
Open

Add socketserver running property #58583

giampaolo opened this issue Mar 20, 2012 · 4 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@giampaolo
Copy link
Contributor

BPO 14375
Nosy @terryjreedy, @pitrou, @giampaolo, @vadmium, @matrixise
Files
  • socketserver.patch
  • 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 = None
    created_at = <Date 2012-03-20.16:53:10.704>
    labels = ['type-feature']
    title = 'Add socketserver running property'
    updated_at = <Date 2016-07-19.01:17:48.670>
    user = 'https://github.com/giampaolo'

    bugs.python.org fields:

    activity = <Date 2016-07-19.01:17:48.670>
    actor = 'martin.panter'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = []
    creation = <Date 2012-03-20.16:53:10.704>
    creator = 'giampaolo.rodola'
    dependencies = []
    files = ['24970']
    hgrepos = []
    issue_num = 14375
    keywords = ['needs review']
    message_count = 4.0
    messages = ['156432', '156937', '270764', '270799']
    nosy_count = 5.0
    nosy_names = ['terry.reedy', 'pitrou', 'giampaolo.rodola', 'martin.panter', 'matrixise']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue14375'
    versions = ['Python 3.3']

    @giampaolo
    Copy link
    Contributor Author

    Patch in attachment adds a "running" property to figure out whether the server is running or not.

    Also it raises an exception in case the server has already been started or stopped. IMO such an event should be prevented beforehand as it signals an application error.

    Finally, __repr__ has been modified in order to reflect the current server status.

    @giampaolo giampaolo changed the title Add socketserver.running property Add socketserver running property Mar 20, 2012
    @bitdancer bitdancer added the type-feature A feature request or enhancement label Mar 20, 2012
    @terryjreedy
    Copy link
    Member

    (I have not used socketserver so my response is somewhat theoretical.)

    .__running seems partly if not completely redundant as the negation of .__is_shutdown. However, I do not see that exposed. I suspect that this was not thought to be needed because the api design is that the user tells the server what state to be in without worrying about what state it is in. File objects have a .closed attribute, but they cannot be reopened when they are, to make sure that they are. A .shutdown attribute might be added.

    Is the server status actually trivalent? new (never started), running, shutdown? If so, perhaps there should be one trivalent status attribute.

    I disagree that telling the server to be in the state it is already in *is* an error. Whether or not to make it an error is a design philosophy and an api choice. For instance, closing a closed file is not an error. In any case, changing the api design is a change, one that could break code. So it would require a compelling reason, a deprecation warning, and a deprecation period. Without a compelling reason stronger than 'IMO', I think that part of the request should be rejected.

    Making an exception part of the api of .server_activate, which explicitly 'May be overridden.', would impose a requirement on overriding methods in subclasses, including those already written.

    The doc for RuntimeError says "(This exception is mostly a relic from a previous version of the interpreter; it is not used very much any more.)". I believe the usual practice is to define a module-specific exception subclass. There is not one now because the current api philosophy does not need one.

    The representation of a file does not include is open/closed status. On the other hand, there is the .closed attribute. So if no attribute is added, putting something in the representation might be done.

    @matrixise
    Copy link
    Member

    what's the status of this issue ?

    @vadmium
    Copy link
    Member

    vadmium commented Jul 19, 2016

    Some responses to the issues raised by Terry would be helpful, especially the incompatibility by adding the RuntimeError. It looks like it forces a race on the normal usage of shutdown(). See also bpo-12463: it seems different people have different ideas about what this method should do.

    Without understanding the motivation(s), I can’t really suggest anything better.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @iritkatriel iritkatriel added the stdlib Python modules in the Lib dir label Nov 27, 2023
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants