Skip to content

Added HTTP Error Code 418 I'm a Teapot#198

Merged
mnot merged 4 commits into
mnot:masterfrom
boramalper:master
Aug 12, 2017
Merged

Added HTTP Error Code 418 I'm a Teapot#198
mnot merged 4 commits into
mnot:masterfrom
boramalper:master

Conversation

@boramalper
Copy link
Copy Markdown
Contributor

redbot fails to implement the 418 I'm a Teapot status code.

Its source is RFC2324, Hyper Text Coffee Pot Control Protocol (HTCPCP/1.0).

Please consider adding support for 418 from Node, since The Internet Engineering Task Force is officially working on reserving 418.

Thanks,

/cc @mnot

(Relevant PR for mod_python.)

@mnot
Copy link
Copy Markdown
Owner

mnot commented Aug 12, 2017

That's not the right place; you're looking for redbot/message/status.py.

@boramalper
Copy link
Copy Markdown
Contributor Author

Thanks for the fast reply, does the new commit solve the issue?

Copy link
Copy Markdown
Owner

@mnot mnot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start.

Comment thread redbot/message/status.py Outdated
class STATUS_IM_A_TEAPOT(Note):
category = categories.GENERAL
level = levels.INFO
summary = "Teapot is requested to brew coffee."
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be more descriptive of the situation; this status code isn't a part of HTTP, so it should say that.

Also, there needs to be a text (as per other Notes) explaining in more detail. The abstract of draft-nottingham-thanks-larry is a good starting point.

Comment thread redbot/message/status.py Outdated

class STATUS_IM_A_TEAPOT(Note):
category = categories.GENERAL
level = levels.INFO
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say levels.BAD (see below)

@mnot
Copy link
Copy Markdown
Owner

mnot commented Aug 12, 2017

... or levels.WARN, I suppose.

@mnot
Copy link
Copy Markdown
Owner

mnot commented Aug 12, 2017

(oh, and don't worry about the CI failure; that's a Python bug, see #189)

@boramalper
Copy link
Copy Markdown
Contributor Author

I was not quite sure of what to write but hope this helps. =)

@mnot
Copy link
Copy Markdown
Owner

mnot commented Aug 12, 2017

That's pretty good! I'm getting sleepy now, but will get it in tomorrow. Thanks!

@boramalper
Copy link
Copy Markdown
Contributor Author

You're welcome. I came here to have some little fun but since you were so helpful and kind, I'm glad that I managed to contribute to your project hopefully in a positive way. =)

Cheers!

Copy link
Copy Markdown
Owner

@mnot mnot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more thing.

Comment thread bin/webui.py Outdated
415: apache.HTTP_UNSUPPORTED_MEDIA_TYPE,
416: apache.HTTP_RANGE_NOT_SATISFIABLE,
417: apache.HTTP_EXPECTATION_FAILED,
418: apache.HTTP_IM_A_TEAPOT,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to take this out; AFAICT your patch to mod_python has been accepted, but they haven't released yet, so this would cause errors for anyone using mod_python until it's available. Also, REDbot (for better or worse) doesn't actually generate 418, so this won't ever be used.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@mnot mnot merged commit 41ae5a0 into mnot:master Aug 12, 2017
@mnot
Copy link
Copy Markdown
Owner

mnot commented Aug 12, 2017

Thanks! I'll get it deployed shortly.

@mnot
Copy link
Copy Markdown
Owner

mnot commented Aug 13, 2017

Should be up.

@boramalper
Copy link
Copy Markdown
Contributor Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants