trio-websocket icon indicating copy to clipboard operation
trio-websocket copied to clipboard

make CloseReason an Enum?

Open belm0 opened this issue 7 years ago • 4 comments

i.e. enum with code as value, and a reason attribute

I'd like to be able to compare close reason without resorting to string comparison

belm0 avatar Oct 17 '18 12:10 belm0

I'd like this as well. There's already an enum in wsproto that could be reused: https://github.com/python-hyper/wsproto/blob/master/wsproto/frame_protocol.py#L85

Caveat: there are valid close codes that do not have names, so there still needs to be a way to instantiate instances with just a code number and optional reason string.

mehaase avatar Oct 17 '18 15:10 mehaase

It sounds like a good reason for trio-websocket to maintain its own enum. (Plus wsproto enum doesn't have reason string.)

Is anything of wsproto exposed currently?

belm0 avatar Oct 17 '18 18:10 belm0

Nothing in wsproto is part of the public trio-websocket API—except for the imported modules that are set to be hidden when the library is re-organized.

mehaase avatar Oct 17 '18 18:10 mehaase

I looked at this again:

  • trio-websocket CloseReason has many-to-one mapping to names like RFC_RESERVED. It's not enum-like. I suggest CloseReason have helper methods or properties for testing "is_rfc_reserved", "is_private_reserved", "is_valid", etc.
  • trio-websocket CloseReason is actually tuple of reason code and arbitrary message (and secondarily, a library-defined string name which may be used for matching, but probably shouldn't-- which is why we want enums). In this regard it's quite different than wsproto CloseReason.

Proposed:

  • CloseReason keeps code and reason message, but drops name property
    • code property remains type int, since it can be any number
    • add helpers for "is_rfc_reserved" etc.
    • in __str__(), attempt to convert code to a new enum (below) before formatting
  • add new CloseCode IntEnum, essentially mirroring wsproto CloseReason

I'm not a big fan of IntEnum due to str() gotchas, but it seems suitable in this case since we want users to easily compare the int code property with named values.

We could also consider having code being type [int | CloseCode], where CloseCode is still IntEnum. But as a user I think I'd rather have a simple, single type.

belm0 avatar May 21 '19 00:05 belm0