Feature handlers#114
Open
alowrydi wants to merge 7 commits into
Open
Conversation
…integration test (OS-assigned port, self-cleaning)
…ndler's result without changing it
DIReview Summary2 critical | 4 warning(s) | 0 suggestion(s)
|
…isn't uninstalled once installed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Central
.z.*handler registry, di.handlersExtracts
code/handlers/dotz.q's handler capture-and-install mechanism from TorQ and packages it as a standalone kdb-x module - the single sanctioned place a process assigns.z.*connection-lifecycle callbacks, replacing the per-file, load-order-dependent closure wrapping TorQ uses today, where whichever handler loads last silently overwrites whoever set the event before it.The other nine files in
code/handlers/-writeaccess.q,controlaccess.q,permissions.q,trackclients.q,trackservers.q,logusage.q,zpsignore.q,ldap.q,apidetails.q- were read as research evidence during design, not as source material this module extracts or consolidates. Their actual content belongs to other modules per the modularisation plan:controlaccess.q/permissions.q/writeaccess.q/ldap.qtodi.permissions,trackclients.q/trackservers.qtodi.clienttracking,logusage.qtodi.querylog,apidetails.qtodi.api(zpsignore.qremains unassigned in the plan). Reading all nine directly - not a subset - is what surfaced the real distinction this module is built on: some of TorQ's.z.*handlers fan out safely across multiple files, others are hard-coded to a single owner with real mutual-exclusion guards between them, and no single file shows both patterns at once.Trello ticket - https://trello.com/c/lMDyVar0/119-kdb-x-handlers-module
Files created
di/handlers/init.qhandlers.q, defines export of 7 functionsdi/handlers/handlers.qdi/handlers/handlers.mddi/handlers/test.csvdi/handlers/test_integration.csvHow to test
Unit tests:
39
true+ 22failassertions, 0 failures.Integration tests (spawns a real second q process on an OS-assigned ephemeral port; skips cleanly if no usable q binary is found, no configuration required).
moduletestonly ever loadstest.csv, so the integration suite is loaded and run directly:3/3 passing. (The
sh: kill: No such processline sometimes seen during this run is expected, harmless output from the belt-and-braces forceful cleanup step finding the child already self-exited via its own.z.pcbackstop - not an error.)Unit test coverage includes: observer fan-out order and isolation on throw, the captured original always running last, duplicate-name replace-in-place, decider single-owner claim/reclaim/reject, decider
removerestoring the kdb-x built-in default via\x,.z.phrejected outright with a self-contained error, dependency validation (all four failure modes),.z.pwpassword redaction to observers (with the owner still receiving the real password), owner-throw stopping the chain before any observer runs,registerreclaim afterobserveleaving the dispatcher installed, full state reset when an observed owner is removed, and init idempotency preserving live registrations across a secondinitcall.Integration test coverage confirms a real synchronous IPC round-trip works while the module is loaded, that a registered
.z.pcobserver fires on a genuine connection close (not just a synthetic call), and that the spawned child process is fully cleaned up afterward - proven against both the normal path and a deliberately forced mid-suite failure, since the child is instructed to self-exit whenever its connection to the test process drops, for any reason, rather than relying on the test process shutting down gracefully.Design decisions
1. Observer vs decider split, not a uniform chaining rule - Not every
.z.*event behaves the same way when more than one thing wants to hook it. For.z.pc,.z.po,.z.exit,.z.wo,.z.wc, kdb-x discards the return value, so any number of registrants can safely coexist -registerfans out to all of them, each individually isolated (a throw is caught, logged atwarn, and doesn't stop the rest). For.z.pg,.z.ps,.z.pi,.z.pp,.z.pw,.z.ws, kdb-x uses the return value as the actual outcome, so only one thing can coherently own it -registerenforces single ownership, with same-name reclaim (needed for idempotent re-init) and different-name rejection naming the current owner. This split was derived from reading all of TorQ'scode/handlers/files directly, not assumed - real TorQ resolves decider conflicts three incompatible ways depending on which files are involved (permissions.q's hard mutual-exclusion exit againstcontrolaccess.q,ldap.q's AND-composition,zpsignore.q's conditional bypass), sodi.handlersdoesn't invent a generic composition rule for deciders - it enforces exclusivity instead. Independently corroborated by KX's own shipped Delta Platform product (.ch.addPC/.ch.addPO/.ch.addExit), which formalises multi-handler chaining for exactly the observer-style events and stops there.2.
.z.phexplicitly out of scope, not silently accepted - On any current kdb-x version,permissions.qdoesn't gate HTTP GET via.z.phat all - it sets.h.val:reqinstead, a completely different hook outside.z.*entirely. Registering against.z.phwould silently do nothing on a real deployment, soregister/observeboth reject it outright with a self-contained error rather than accepting it as a normal decider.3.
observe/unobserve- side-effect-only watchers on a decider's owner - Single ownership on decider events is correct and unchanged, but real TorQ proves a decider event also needs pure watchers alongside its owner -logusage.qwraps every decider event with log-before/after logging that passes the result through untouched;trackclients.qwrapspg/ps/wswith byte-counting the same way.observe[event;name;func]attaches a watcher to a decider event that already has an owner; it runs after a successful owner call, receivesfunc[result;args], and can never alter what the caller actually receives - the dispatcher's own return statement is the owner's result, computed before any observer runs, so nothing an observer does or returns can reach the client. For.z.pwspecifically, the owner receives the real password (it has to, to authenticate) but observers only ever receive(user;"***")- modelled directly onlogusage.q's own real redaction of the same field.4. OS-assigned ephemeral port for the integration test, no configuration - The integration suite needed a real second process on a real port. Rather than a fixed range or an environment variable, the test asks the OS directly (
\p 0W, confirmed against kdb-x's own documented ephemeral-port mechanism) and reads back whatever port the kernel actually assigned - guaranteed free at the moment of assignment, no configuration step, no bespoke variable. Cleanup is robust to a genuine mid-suite crash, not just the happy path: the spawned child is told to self-exit the instant its connection to the test process drops, for any reason, which was verified empirically to survive an uncaught throw partway through the suite, not just assumed to work.5. Export list reviewed against the public API criterion - All seven exports (
init,register,remove,list,version,observe,unobserve) are genuine consumer-facing functions. The dispatch machinery, the per-event state, and the decider-observer internals are all private to the module.Checklist
consistency.mdandstyle.mdhandlers.mddocuments all exported functions, config, usage examples and notesdi.*modules - standaloneuseimports anywhere in the moduleDocumentation
See
handlers.mdfor full reference including the managed-events table, dependency table, exported function documentation with examples, usage example, and notes.