Opened 11 years ago

Closed 11 years ago

#2273 defect closed fixed (fixed)

AMP documentation on command errors is reversed or code is

Reported by: htj Owned by:
Priority: normal Milestone: Core-2.5
Component: core Keywords: amp
Cc: Branch:


line 1038 in protocols/ says:

@cvar errors: A mapping of error tag to exception type.

which is wrong because errors have to be mapped like this to work:

errors = {SomeException: 'SOME_ERROR'}

The easy solution is to flip the text in the docstring so it reads:

@cvar errors: A mapping of exceptions types to error tags.

However, I think that what the docstring says is better. By having ERROR_TAG -> Exception, it is possible to map several error tags to the same exception. This is not possible with the current scheme, as an exception maps to an error tag. Hence one exception cannot map to several tags, unless we start using lists, but that just makes it more nasty.

I know this is an API change, but since AMP has not yet been in official release it should be doable.

Change History (6)

comment:1 Changed 11 years ago by Jean-Paul Calderone

Milestone: Twisted-2.5

comment:2 Changed 11 years ago by Glyph

Owner: changed from Glyph to radix

You should do something about this, either changing the docs or the code.

comment:3 Changed 11 years ago by radix

Keywords: review added
Owner: changed from radix to Tristan Seligmann

Ready for review in fix-amp-docstring-2273

For the reviewer's convenience, I include the entire changeset here:

-    @cvar errors: A mapping of error tag to exception type.
+    @cvar errors: A mapping of exception type to error tag.

comment:4 Changed 11 years ago by Tristan Seligmann

Keywords: review removed
Owner: changed from Tristan Seligmann to radix

As best I can tell, this change is correct. Go ahead and merge.

comment:5 Changed 11 years ago by radix

Resolution: fixed
Status: newclosed

(In [18850]) Merge fix-amp-docstring-2273

Author: radix Reviewer: mithrandi Fixes #2273

One is meant to pass a mapping of exception types to tags as the 'errors' argument to Command, not a mapping of tags to exception types.

comment:6 Changed 6 years ago by <automation>

Owner: radix deleted
Note: See TracTickets for help on using tickets.