Repository navigation
design: should the non-registry helper enums share a lookup interface, or be made strictly immutable? #877
Description
Activity
- added
on Sep 28, 2026 Clarification from the owner, verbatim:
and for my "get/register-only, no unregistered/unminted value reutnr, nor missing/aliasing hanalders", it means, the helper enum will ONLY have get and register logic; it will not return a unregistered and unminted value from
_missing_or get but rather directly raise; and it does not support aliasing (up to discussion)That retracts my "
get/register-only is self-contradictory" objection, and I was wrong to raise it. The distinction is between deliberate, caller-initiated registration and implicit minting on a lookup miss.registeris the former; #775's defect class is the latter. A base withget+registerthat raises on a miss is internally consistent.Two measurements now decide the shape.
1. The raise-on-miss property does not live in
get— it lives in the subclass's_missing_. Measured on a real registry:LinkType.get(999999) -> <LinkType.Unassigned: 999999> # unregistered member, via cls(key) -> _missing_ LinkType.get('NoSuchName') -> KeyError: 'NoSuchMember'getcallscls(key), which goes through whatever_missing_the generated subclass defines. So "does not return an unregistered value" is obtained by not defining a permissive_missing_, not by slimming the base — a helper that omits it already raises today.2. The real obstacle is case folding, and it breaks in whichever direction we choose.
TransportProtocol.get('TCP') -> tcp # helper: key.lower(), case-INsensitive LinkType.get('ETHERNET') -> ETHERNET # base: cls._member_map_[key], case-SENSITIVE LinkType.get('ethernet') -> KeyErrorPutting
TransportProtocolon the base as-is breaksget('TCP'). Either the base gains case folding — a behaviour change across 120 registries — or the helper loses it, breaking its own callers. That is the decision I would want settled before any base is written.Side finding while measuring, worth its own fix regardless:
getraisesKeyErrorfor a name miss andValueErrorfor a value miss, and neither is apcapkit.utilities.exceptionstype, which the house rule requires for in-library errors.On aliasing — you left it open. I would leave it out of the slim base: an alias is a second name for a registered value, which is only meaningful where names come from an external registry, and none of the three helpers has one.
Scope widened by the owner, verbatim:
let's move the conversation to the GH issue (and the scope is not limited the three helper you listed, but actually every single enum defined in the library)
So this is no longer about
CommandType,ConformanceRequirementandTransportProtocol— it covers every enum class defined anywhere inpcapkit/, and the discussion lives here rather than in chat from now on.A repo-wide census is in flight, partitioned so the population is legible rather than a wall of 120 rows:
EnumRegistrysubclasses underpcapkit/const/— count, plus individually only the unusual ones: any that defineget,register,register_aliasor_missing_themselves instead of inheriting, and any whose_missing_departs from the standardreturn cls._unregistered_member(value, '<name>')shape. Those are what a base change could break.EnumRegistrysubclasses outsidepcapkit/const/, if any exist — in full.- Every non-
EnumRegistryenum anywhere inpcapkit/— in full, with base list, member count, and which ofget/_missing_/register/register_alias/register_aliases/_unregistered_member/extend_enumeach defines or calls.
For group 3 the census also records the axis this decision actually turns on: does its
_missing_mint or raise — where "mint" includes delegating to asuper()._missing_that caches, asIntFlagdoes. Worth restating because it already caught me out once on this issue:__members__does not revealIntFlagcomposite caching;_value2member_map_does.The two open questions from the measurements above stand and are now library-wide rather than helper-specific: whether
getshould case-fold (the base is case-sensitive,TransportProtocol.getis not, and one of the two has to change), and whether aliasing belongs in a slim base at all. I will post the census here when it lands, and hold off recommending a shape until the counts are in — a shared base earns its keep at 10 classes and not at 3.Census in, and the population is 21 — not 3. I re-ran the count myself rather than relaying it; my AST scan over all of
pcapkit/reproduces the agent's numbers exactly:EnumRegistry subclasses: 120 non-registry enums: 21 total: 141 non-registry with .get: 8 of those minting: 6The eight
.gets, and this is the finding that reframes the question — six of them mint:class file .getmints?FastBindingAcknowledgmentStatusprotocols/internet/mh.pyyes IPv6AddressPrefixCodeprotocols/internet/mh.pyyes LocalizedRoutingStatusprotocols/internet/mh.pyyes LMAAddressCodeprotocols/internet/mh.pyyes ProcedureCodeprotocols/application/ngap.pyyes ProtocolIEprotocols/application/ngap.pyyes Criticalityprotocols/application/ngap.pyno — raises TransportProtocolconst/reg/apptype/apptype.pyno — raises, and case-insensitively They are not eight one-offs and not one function: six near-identical copies of a minting lookup confined to two files (
extend_enum(X, key, default=-1)then re-subscript), plus two genuine one-offs. The other 13 non-registry enums have no.getat all, and not one of the 21 definesregister,register_alias, or calls_unregistered_member.The shared interface (a)/(c) would create has no consumer. The two places that genuinely treat helper and registry enums interchangeably already share one, inherited from
Enum:ProtocolBase._make_index(protocols/protocol.py, ~117namespace=call sites) usesnamespace[name], andEnumField(corekit/fields/numbers.py, 124 namespace classes) usesself._namespace(value). Neither would lose a line if all 21 gainedget/register.Two free subtractions available today:
LocalizedRoutingStatus.getandLMAAddressCode.gethave zero callers anywhere in the repo — I grepped, including tests. Dead API that advertises a minting path nobody uses.Where that leaves the three options. (a) would re-open minting on 13 currently-closed enums and hand
register_aliasto things likeFinalisedStateandCompletion. (c) cannot host the six copies that are the only duplication worth removing, since by your definition it forbids minting — and of the two it could host,TransportProtocolis under a standing ruling against ever extending it, so (c) ends up a public base for one willing caller. (b) is the coherent one, but it is the most expensive to land, because the minting is pinned by live assertions (test_mh_unit.py:1530,:1534assertget('Vendor_specific', 200) == 200), not merely tolerated.The strongest argument against (b), stated plainly: it leaves two permanent, contradictory answers to "what does
.getmean on a pcapkit enum" — never mints on the 120 registries, mints on six helpers — plus the case-folding fork, in an API that presents as uniform. If one meaning forgetacross the package outweighs adding an abstraction for eight classes, then the answer is a slim base that permits an opt-in minting policy, and (b) is wrong.One unmeasured lead worth a check before it matters:
mh.py's shape mints withdefault=-1, so two different unknown keys would both be extended at value-1, andaenumtreats a duplicate value as an alias request — the second key may resolve to the first key's member. Inferred fromEnumRegistry.register's own docstring, not measured; I can settle it in three lines if you want it as a defect rather than a footnote.Correction to my own census above: the registry population is 124, not 120. I verified it at runtime by walking every module and testing the MRO rather than the base list:
carrying EnumRegistry (MRO): 124 direct base: 120 transitive: 4 -> reg/apptype/{tcp,udp,sctp,dccp}.py::{TCP,UDP,SCTP,DCCP} outside pcapkit.const: 0Those four inherit
AppType, which mixes in the base, so they carry the full interface from a different file — which is exactly why a base-list AST scan misses them. They were counted in neither of my earlier buckets, so the honest totals are 145 enum classes: 124 carrying the registry interface, 21 not.EnumRegistry's own module docstring says 124, and it was right.Two further facts that matter to this decision:
The registry protocol has never leaked out of the generated-constants layer — zero
EnumRegistrysubclasses anywhere outsidepcapkit/const/, across 507 modules. That is evidence about demand, not just about current structure, and it cuts against extending the protocol toprotocols/-layer helpers.Three registries still mint directly from
_missing_viaextend_enum—const/ipx/socket.py::Socket,const/mh/cga_type.py::CGAType,const/reg/ethertype.py::EtherType— kept deliberately per #775's ruling. Note two of those three are precisely what #775's pending branch converts, leavingCGATypeas the standing carve-out. So a base change scoped against "no registry mints" would be wrong today and right after that lands.One unreconciled number, flagged rather than papered over:
tests/const/test_const_enum_no_mint.py:829speaks of "every one of the 105". 105, 120 and 124 are three different populations in this tree and I have not reconciled which the test means. Anyone scoping a base change should settle that first — using the wrong one of the three is how a registry gets missed.The group-3 mint/raise split, restated with
CommandTypequalified: 7 minting, 1 raising (Criticality), 13 closed with no_missing_at all.CommandTypesits on the mint side only in the table-growth sense — it delegates toIntFlag._missing_, which caches a composite in_value2member_map_while leaving__members__untouched; I measured that growth earlier on this thread (5 → 7), and you have already ruled it expected.My initial thought was to make them immutable - unless RFC/IANA says otherwise. Therefore they may subclass a bare base enum from pcapkit.corekit.enum - where EnumRegistry subclasses it for using in the other mutable ones.
Then there's a need to audit if based on RFC/IANA/IETF documentations and industry conventions, such an enum is extensible/dynamic/mutable then decide which base class it inherits from.
Also, if one enum is recorded in a IANA CSV format or any parsable-table-like data format, we should consider moving them to become generated by vendor crawlers rather than hardcoded and hand maintained.
Recorded, verbatim:
My initial thought was to make them immutable - unless RFC/IANA says otherwise. Therefore they may subclass a bare base enum from pcapkit.corekit.enum - where EnumRegistry subclasses it for using in the other mutable ones.
Then there's a need to audit if based on RFC/IANA/IETF documentations and industry conventions, such an enum is extensible/dynamic/mutable then decide which base class it inherits from.
Also, if one enum is recorded in a IANA CSV format or any parsable-table-like data format, we should consider moving them to become generated by vendor crawlers rather than hardcoded and hand maintained.
So the shape is settled: a bare base in
pcapkit/corekit/enum.py, withEnumRegistrysubclassing it — parent, not sibling — and immutability as the default that an external registry has to earn. That also resolves theget/registerquestion from earlier: lookup belongs on the bare base, mutation stays inEnumRegistry.needs: decisioncomes off; this becomes tracking work in three parts.1. The bare base.
getandget_allmove down;register,_extend,register_alias,register_aliases,_unregistered_memberstay up. Measured earlier:get's body touches only_member_map_,_value2member_map_,cls(key)and theNO_DEFAULTsentinel, so it needs nothing from the registry concept. Two open sub-questions that the base's signature has to settle rather than inherit by accident — whethergetcase-folds (the base is case-sensitive today,TransportProtocol.getis not, and one of the two must change), and thatgetcurrently raisesKeyErrorfor a name miss andValueErrorfor a value miss, neither frompcapkit.utilities.exceptions.2. The audit, which gates everything else. 21 non-registry enums, of which 8 carry a
.getand 6 of those mint. Each needs a documented verdict — extensible per RFC/IANA/IETF or a closed set — with the citation recorded, not inferred from its name. An audit is now in flight and will report per class: the governing document, whether the value space is open, which base it should take, and whether any current behaviour has to change to match. Note two of the 21 (LocalizedRoutingStatus.get,LMAAddressCode.get) have zero callers repo-wide, so for them the answer may simply be deletion.3. The crawler question. Any enum whose source is an IANA CSV or another parsable table is a candidate to move from hand-maintained to generated. The audit will flag which of the 21 have such a source; that will be its own issue, since it is a different kind of change from the base split and should not ride along with it.
Audit in. Per-class verdicts against the ruling, with the citation for each read from the source rather than inferred. Two corrections to my own earlier numbers first.
The population is 26, not 21. The base-list AST scan missed nested enum classes. Measured — 7 nested
Flagclasses inprotocols/schema/application/httpv2.py, none inheritingEnumRegistry:FrameType.Flags 0 members (deliberately memberless abstract base) DataFrame.Flags HeadersFrame.Flags SettingsFrame.Flags PushPromiseFrame.Flags PingFrame.Flags ContinuationFrame.FlagsThe six concrete ones are closed sets fixed by the HTTP/2 frame definitions and want the same treatment as
CommandType— a base preservingFlagsemantics.FrameType.Flagshas no value space at all (FrameType.Flags(0)raisesTypeError: <flag 'Flags'> has no members, intentional and documented athttpv2.py:148-158), so it is out of scope rather than closed.The verdicts. 12 rows are pure re-parenting with no behaviour change —
FinalisedState,PacketDirection,PacketReception,WireGuardKeyLabel,ESPStatus, bothTypes,PDUKind,Criticality,ConformanceRequirement,Completion, plus the 6httpv2Flags. They already raise and carry no registry API.Four contradictions, all in
mh.py—FastBindingAcknowledgmentStatus,IPv6AddressPrefixCode,LocalizedRoutingStatus,LMAAddressCode. Each mints today while its own docstring says the opposite, e.g.mh.py:576: ":rfc:5568#section-6.2.3defines these values inline and IANA keeps no registry of them, so the enumeration lives here…" Closed registries that mint. Theirgetalso carries a defect I have filed separately as #880 — a shared-1sentinel that aliases the second unknown key to the first, and installs a member the class's own0 <= value <= 255guard rejects. Fix #880 first so the two changes stay separable;LocalizedRoutingStatus.getandLMAAddressCode.gethave zero callers and can simply go.Two correctly open:
ProcedureCodeandProtocolIE— 3GPP TS 38.413,INTEGER (0..255)and(0..65535), 81 and 438 named.EnumRegistryis right for them, and they are also the strongest candidates for your crawler point — but their source is an installed Python dict (pycrate_asn1dir/NGAP.py), not an HTTP-fetchable CSV, so it does not fit the existingVendor.LINKshape.ngap.py:169-171says as much today. That is a design decision, not a mechanical port.One needs your call:
TLSKeyLabel(pcapng.py:273). It cites nothing, and its real source is the NSSSSLKEYLOGFILEformat, whose label set grows as TLS features land — convention says open, the ruling's default says immutable, and there is no citation to settle it.Two to leave alone:
TransportProtocolandCommandTypeare already settled by your own rulings quoted in-source (apptype.py:105, and pinned attest_const_enum_no_mint.py:1959) and already behave as the ruling wants.TransportProtocolis referenced by 15 test files and 29 source files — the widest blast radius in the population — so re-parenting it is cosmetic and expensive.Step 0 is that the bare base does not exist yet:
pcapkit.corekit.enum.__all__is['NO_DEFAULT', 'NoDefaultType', 'EnumRegistry']. Creating it and re-basingEnumRegistrychanges the MRO of all 124 registry enums, so it should land alone, with nothing else in the change.Two measured traps for whoever writes the "immutable" test:
CommandType._value2member_map_is not fixed — it holds6: P|Sat import and gains5: A|Son firstCommandType(5)while__members__stays at 4 — and the same applies to the 6httpv2Flags, forFlagreasons that are not minting. Alsotest_const_enum_no_mint.py:1974assertsassertFalse(issubclass(CommandType, EnumRegistry)), which needs re-reading against any new base.Not verified: rows 2-3 (
PacketDirection,PacketReception) rest on bit-widths derived from their member values, not on the pcapng draft's text — no network was used, so someone should check §4.3epb_flagsbefore acting on those two. No test file was run, so every "tests would break" claim is reference-mapping, not an observed failure.For ProcedureCode and ProtocolIE, since their source is from
pycreate, we can directly 1) read from the Python object at runtime; or 2) read from its GitHub or PyPI distributed sources. We had Vendor.DATA for that.For TLSKeyLabel, what's your recommendation? We should work aiming for future. So I guess it should stay open?
For TransportProtocol and alike, reparenting anyways - we're enforcing and establishing a convention here.
Any additional items I should make a ruling?
Recorded, verbatim:
For ProcedureCode and ProtocolIE, since their source is from
pycreate, we can directly 1) read from the Python object at runtime; or 2) read from its GitHub or PyPI distributed sources. We had Vendor.DATA for that.For TLSKeyLabel, what's your recommendation? We should work aiming for future. So I guess it should stay open?
For TransportProtocol and alike, reparenting anyways - we're enforcing and establishing a convention here.
Any additional items I should make a ruling?
On
Vendor.DATA— one correction, because it changes which of your two options is the real win. There is noDATAattribute onVendor;grep -n DATA pcapkit/vendor/default.pyreturns nothing. It is a per-crawler convention: a module-levelDATAdict plus arequest()override returning it, used by at least 8 crawlers (pcapng/secrets_type.py,pcapng/verdict_type.py,ipx/socket.py,tcp/flags.py, …). So option (2) — vendoring pycrate's table as aDATAliteral — would move the hand-maintenance from the const file into the vendor file without removing it. Option (1) is the one that actually automates: arequest()that imports pycrate at generation time and readsNGAP_Constantslive. Cost to weigh: it makes pycrate a generation-time dependency, and the emitted table then varies with whichever pycrate the generator ran against — which wants recording in the generated header.TLSKeyLabel: yes, open — my recommendation, and it costs nothing structurally. The NSSSSLKEYLOGFILElabel set has already grown once within this very class (RSA/CLIENT_RANDOMare the SSL-era pair; the other seven are TLS 1.3 secrets), and pcapng's secrets block merely carries the label text, so an unrecognised label is a decode-time fact rather than an error. And it needs no new machinery: 5 registries already mixEnumRegistrywithStrEnum(measured — 110IntEnum, 5StrEnum, 5IntFlag), so this is a precedented shape, not a first.TransportProtocoland alike: understood, reparenting all of them. That overrides the audit's "leave alone" recommendation forTransportProtocolandCommandType— convention beats blast radius, and the radius is test-file churn rather than behaviour.Five things I would still want a ruling on, in the order they block work:
- The four
mh.pyenums — the decision that unblocks fix: six .get implementations mint unknown keys at a shared -1, so the second aliases to the first #880. They cite "IANA keeps no registry of them" yet each reads an 8-bit wire field, so an unnamed value is something a real capture can carry. Should_missing_raise, or return a non-registering_unregistered_member(closed registry, tolerant parser)? I lean tolerant, since a parser that raises on a byte it merely does not recognise is worse than one that labels it. - Does
getcase-fold? The base is case-sensitive (LinkType.get('ethernet')→KeyError) whileTransportProtocol.get('TCP')succeeds by lowercasing internally. Reparenting forces a choice, and either direction breaks a caller. get's exception types —KeyErrorfor a name miss,ValueErrorfor a value miss, and neither frompcapkit.utilities.exceptionsas the house rule requires. Worth fixing while the base is being written.- The six nested
httpv2Flags— same convention presumably, but note their_value2member_map_grows byFlagcomposite caching without minting, asCommandType's does. Any "immutable means the map never grows" test will fail on all seven for reasons that are not minting. PacketDirection/PacketReception— I marked them closed from their member values, not from the pcapng draft's text (no network was used). Someone should read §4.3epb_flagsbefore they are declared closed, since "reserved" and "undefined" point different ways.
- The four
53 remaining items
- added 13 commits that reference this issue
on Oct 2, 2026
Follow-up from #860, moved here so the discussion has its own thread. 120 of 123 const enum classes inherit
EnumRegistry; the three that do not areftp/command.py::CommandType(IntFlag),ftp/command.py::ConformanceRequirement(IntEnum) andreg/apptype/apptype.py::TransportProtocol(IntEnum).The owner's framing, verbatim:
So: (a) helpers onto
EnumRegistry; (b) strip their.getand any minting so they stay immutable; (c) a slimmer base carrying only the shared auxiliaries.Three measurements that constrain the answer.
EnumRegistryis exactly seven classmethods —get,get_all,register,_extend,register_alias,register_aliases,_unregistered_member— and defines no_missing_: those hooks live in the generated const classes, so "no_missing_handler" is not something a new base has to avoid.getis already registry-free. Its body touches onlycls._member_map_,cls._value2member_map_,cls(key)and thedefaultsentinel; it never calls_unregistered_memberand never mints. It is generic enum sugar, and strictly richer than the helpers' hand-rolled versions (name lookup and value lookup and adefault).get/register-only is self-contradictory, though.registerguards a duplicate then calls_extend, and_extendis the mutation — registering is minting, deliberate rather than accidental. A base that registers is not immutable.And a slim variant cannot be a subclass of
EnumRegistry. Inherited methods cannot be removed, and overriding_unregistered_memberto raise yields a class advertising the method it forbids. The coherent shape is a parent:EnumLookupcarryingget/get_all, thenEnumRegistry(EnumLookup)adding the mutating half.That makes (c) viable only along the
getline, where it converges with (b): if the helpers keep just a lookup, the only question left is whether that lookup is shared or duplicated. A repo-wide census — how many non-registry enums exist outsidepcapkit/const/, and whether their.gets are the same function modulo names — is in flight and will decide it; a shared base earns its keep at 10 classes and not at 3.