skupperproject / skupper-router

An application-layer router for Skupper networks
https://skupper.io
Apache License 2.0
14 stars 18 forks source link

Issue #1498: Initial HTTP/1.x observer implementation #1513

Closed kgiusti closed 1 month ago

astitcher commented 1 month ago

I was perhaps a bit too terse earlier.: The usual python recommendation is to use Enum rather than IntEnum if you can unless you have back compatibility constraints. I see you used IntEnum, but I can't tell if you could have equally easily used a plain Enum whiich recommended for new code. Sorry to be a pain!

kgiusti commented 1 month ago

I was perhaps a bit too terse earlier.: The usual python recommendation is to use Enum rather than IntEnum if you can unless you have back compatibility constraints. I see you used IntEnum, but I can't tell if you could have equally easily used a plain Enum whiich recommended for new code. Sorry to be a pain!

Yeah I'd rather keep it as IntEnum. Changing it to Enum requires explicit use of the .value attribute:

>>> from enum import Enum
>>> class AttributeTypes(Enum):
...     RECORD_TYPE = 0
...     IDENTITY = 1
...     PARENT = 2
... 
>>> AttributeTypes.IDENTITY
<AttributeTypes.IDENTITY: 1>
>>> AttributeTypes.IDENTITY.value
1
>>> dd = {1: 'a', 3: 'b', 0: 'c'}
>>> dd[1]
'a'
>>> dd[AttributeTypes.IDENTITY]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
KeyError: <AttributeTypes.IDENTITY: 1>
>>> dd[AttributeTypes.IDENTITY.value]
'a'

Whereas I find

dd[AttributeTypes.IDENTITY]

a bit clearer.