classification
Title: Add an http method enum
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.6
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: ethan.furman Nosy List: demian.brecht, ethan.furman, martin.panter, matrixise, r.david.murray, rhettinger, terry.reedy
Priority: normal Keywords: needs review, patch

Created on 2016-02-18 06:24 by demian.brecht, last changed 2016-07-18 21:50 by ethan.furman. This issue is now closed.

Files
File name Uploaded Description Edit
issue26380.diff demian.brecht, 2016-02-18 06:28 review
issue26380_1.diff demian.brecht, 2016-02-18 19:22 review
issue26380_2.patch demian.brecht, 2016-02-18 23:53 review
Messages (13)
msg260431 - (view) Author: Demian Brecht (demian.brecht) * (Python triager) Date: 2016-02-18 06:24
Super minor annoyance that I've had over multiple projects is seeing either hard coded strings being used (which is a bit of a no-no in terms of best practices) or each project defining its own set of constants for http methods. Why not just include a standard set in http as is done for status codes?
msg260433 - (view) Author: Demian Brecht (demian.brecht) * (Python triager) Date: 2016-02-18 06:28
If nobody's opposed to the addition, I'll run through the unit tests and replace the hard coded strings.
msg260465 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2016-02-18 16:17
I don't have a firm opinion at this point -- can you give a few examples of how this will help in code?
msg260479 - (view) Author: Demian Brecht (demian.brecht) * (Python triager) Date: 2016-02-18 19:22
> I don't have a firm opinion at this point -- can you give a few examples of how this will help in code?

It'll help solely with consistency across projects. Generally, using constants are generally favored over using hardcoded values. Mainly, it helps reduce typos. Some projects use literals, others use project-specific constants. It's something that I've found myself redefining over various projects, and I just though "why couldn't this be added to the standard library, given it's an attribute of the HTTP/1.1 RFC"?

/Very/ minor, personal annoyance.

The updated patch fixes the OPTION typo.
msg260482 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-02-18 21:24
I can see the advantage of using an enum over a plain string. But you only get an error at run time, not compile time, if you misspell it. And there is also the disadvantage of the extra boilerplate of importing HTTPMessage. So I don’t have a strong opinion either way. Do other libraries have a similar enum?

BTW I think it is actually OPTIONS; see review comment.
msg260488 - (view) Author: Demian Brecht (demian.brecht) * (Python triager) Date: 2016-02-18 23:49
> I can see the advantage of using an enum over a plain string. But you only get an error at run time, not compile time, if you misspell it.

Sure, but at least you're giving static analysis utilities the chance to catch it up front.

> And there is also the disadvantage of the extra boilerplate of importing HTTPMessage.

I guess that all depends on how you're importing your modules. If you just import "http", then there's no additional boilerplate.

> So I don’t have a strong opinion either way. Do other libraries have a similar enum?

I haven't seen enums in other libraries, only constants. I figured that it might as well be consistent with HTTPStatus, although granted, HTTPStatus does a little more than the methods.
msg260489 - (view) Author: Demian Brecht (demian.brecht) * (Python triager) Date: 2016-02-18 23:53
> BTW I think it is actually OPTIONS; see review comment.
Well, don't I feel silly. Fixed.
msg260547 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-02-20 02:14
If I were deciding, I would be inclined to reject this.  Part of the understanding when enums were added was that they would not automatically be used in the stdlib anywhere there could be used.

To answer 'why not', there is an obvious gain to naming arbitrary numerical code, as with HTTPStatus.xyz.  I do not see any gain from replacing 'GET'*  with HTTPMethod.GET.  Just more work to write and read.

*Does http require uppercase for the methods?

To answer 'best practice',  I disagree with the premise that 'hardcoding' such short meaningful names is bad.  I think that this is a mis-application of a sometimes valid principle.

If people who do like such replacements are changing spellings, other than to lower case the words (enum value do not have to be uppercase), in the process, shame on them. Otherwise, typing HTTPMethod.OPTIONS is harde to type correctly, not easier, than 'OPTIONS'.  If you posted evidence as to your claim, I might be more favorably inclined.  On the other hand, if everyone used the quoted strings, 'GET', etc, there would be no problem with inconsistency.

My personal experience with turning constant strings into constant names is with tkinter, which has about 50 assignments like "E = 'e'", "RAISED = 'raised'", and so on.  I consider them more a nuisance than a help.  CAPS are harder to type than letters, and they give TOO MUCH EMPHASIS to rather minor configuration issues.  If one does 'import tkinter' or 'import tkinter as tk' instead of 'from tkinter import *', then "relief=tk.RAISED" is definitely harder to write as "relieve='raised'", and to me uglier.

To me, your later throw-in comment about static analyzers might be the most persuasive point you made
msg260552 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-02-20 03:08
The RFC for HTTP says the method is case-sensitive, although I have seen one person use lowercase (probably by accident). So “over the wire” it has to be uppercase b"GET".
msg260658 - (view) Author: Demian Brecht (demian.brecht) * (Python triager) Date: 2016-02-22 05:12
> To me, your later throw-in comment about static analyzers might be the most persuasive point you made

Well, we're in agreement there :) As I'd mentioned, at best it's a minor annoyance. I'm entirely on the fence about the use of the enum as well and after reading your response, am more inclined to change this to simple constants as the enum really doesn't add much.
msg270753 - (view) Author: Stéphane Wirtel (matrixise) * (Python triager) Date: 2016-07-18 13:22
what's the status of your issue ?
msg270791 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-07-18 21:33
I concur that the enum adding nothing of value here.
msg270792 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2016-07-18 21:50
That was the last nudge I needed.

Thanks, everyone.
History
Date User Action Args
2016-07-18 21:50:11ethan.furmansetstatus: open -> closed
messages: + msg270792

assignee: ethan.furman
resolution: rejected
stage: patch review -> resolved
2016-07-18 21:33:30rhettingersetnosy: + rhettinger
messages: + msg270791
2016-07-18 13:22:46matrixisesetnosy: + matrixise
messages: + msg270753
2016-02-22 05:12:31demian.brechtsetmessages: + msg260658
2016-02-20 03:08:36martin.pantersetmessages: + msg260552
2016-02-20 02:14:28terry.reedysetnosy: + terry.reedy
messages: + msg260547
2016-02-18 23:53:35demian.brechtsetfiles: + issue26380_2.patch

messages: + msg260489
2016-02-18 23:49:36demian.brechtsetmessages: + msg260488
2016-02-18 21:24:37martin.pantersetnosy: + martin.panter
messages: + msg260482

type: enhancement
stage: patch review
2016-02-18 19:22:14demian.brechtsetfiles: + issue26380_1.diff

messages: + msg260479
2016-02-18 16:17:48ethan.furmansetnosy: + ethan.furman
messages: + msg260465
2016-02-18 06:28:34demian.brechtsetfiles: + issue26380.diff
keywords: + patch
messages: + msg260433
2016-02-18 06:24:04demian.brechtcreate