adding new tplogutils module #78
Conversation
|
jonathonmcmurray
left a comment
jonathonmcmurray
left a comment
There was a problem hiding this comment.
please align all code to style guide at https://github.com/DataIntellectTech/kdbx-modules/blob/main/style.md
(e.g. comment style, naming conventions etc.)
| MAXCHUNK: 8 * CHUNK; / - don't let single read exceed this | ||
|
|
||
| check: {[logfile;lastmsgtoreplay] | ||
| / - logfile (symbol) is the handle to the logsfile |
There was a problem hiding this comment.
fix comment style to align with style guide https://github.com/DataIntellectTech/kdbx-modules/blob/main/style.md
| CHUNK: 10 * 1024 * 1024; / - size of default chunk to read (10MB) | ||
| MAXCHUNK: 8 * CHUNK; / - don't let single read exceed this | ||
|
|
||
| check: {[logfile;lastmsgtoreplay] |
There was a problem hiding this comment.
| check: {[logfile;lastmsgtoreplay] | |
| check:{[logfile;lastmsgtoreplay] |
remove unnecessary space (same for other functions & variables in this file)
| :@[d;`start`size;:;(ns;CHUNK)]; | ||
| }; | ||
|
|
||
| export:([check;repair;repairover]) |
There was a problem hiding this comment.
do we want to export all these? I think end users should probably only run check; if that's the case, that's the only one we should export - possibly also repair, but I definitely think that repairover is an internal function that shouldn't be exported
| {[h;i;t;corruptpos] | ||
| if[=[i;corruptpos]; | ||
| data:enlist (`upd;`trade;value t[i]); | ||
| data_bytes:-18!data; |
There was a problem hiding this comment.
don't use _ in variable names https://github.com/DataIntellectTech/kdbx-modules/blob/main/style.md#naming
| msgcount:10; | ||
|
|
||
| / Setup | ||
| createValidLog[testfile;msgcount]; |
There was a problem hiding this comment.
this function has been renamed, but references not updated - probably true of other functions etc. as well
There was a problem hiding this comment.
do these tests still pass? (I'm pretty sure they won't)
| / ============================================================================= | ||
|
|
||
| / @test Verify module constants are set correctly | ||
| testconstantsset: { |
There was a problem hiding this comment.
I don't think this function is called in tests; if it's not needed (it looks a bit pointless IMO), let's remove it - also we should check for any other functions that aren't called in tests & either remove them or add them to tests (if they're actually useful tests)
| ## :label: Naming note (important) | ||
|
|
||
| The folder name is `tplogutils`, and `init.q` exports: | ||
|
|
||
| ```q | ||
| export:([check;repair;repairover]) | ||
| ``` | ||
|
|
||
| However, the included tests reference the name `tplogsutil` in a few places (e.g. `tplogsutil.check`, `tplogsutil.repair`). | ||
|
|
||
| Depending on your package loader conventions, you may want to: | ||
| - load the module into a variable named `tplogsutil`, **or** | ||
| - update the tests to use `tplogutils` consistently. | ||
|
|
||
| This README uses **`tplogutils`** when referring to the module variable. |
There was a problem hiding this comment.
this is weird & confusing IMO - why are we suggesting that end users update tests or load module with a different name etc.? we should be keeping this simple, we should have everything named correctly by default and remove this section from docs
| ### KDB-X (supports `use`) | ||
| If you are using KDB-X (where `use` exists), load the module using the symbol that matches your `QPATH` layout. | ||
|
|
||
| If your `QPATH` includes the `di` directory (e.g. `~/kdbx-modules/di`), a common pattern is: | ||
|
|
||
| ```q | ||
| tplogutils:use`tplogutils | ||
| ``` | ||
|
|
||
| ### Plain q (no `use`) | ||
| If your session does not support `use`, load directly from the file: | ||
|
|
||
| ```q | ||
| \l /path/to/kdbx-modules/di/tplogutils/init.q | ||
| ``` |
There was a problem hiding this comment.
I think we should assume that anyone using this is using kdbx
| HEADER: 8 # -8!(`upd;`trade;()); / - header to build deserialisable msg | ||
| UPDMSG: `char$10 # 8 _ -8!(`upd;`trade;()); / - first part of tp update msg | ||
| CHUNK: 10 * 1024 * 1024; / - size of default chunk to read (10MB) | ||
| MAXCHUNK: 8 * CHUNK; / - don't let single read exceed this |
There was a problem hiding this comment.
we should probably provide a function for users to set these, with the current values as defaults
I'd also prefer they be named with lowercase names
Description
Create module containing utils for recovering data from corrupt log files
Potential start point:
TorQ TP Log Utils Documentation
TorQ TP Log Utils Code
KDB-X Modules
Acceptance Criteria
Please create a separate directory for each package and place code, documentation and unit tests within. All packages must have documentation and unit tests to be accepted.Package: TP Log File Recovery · DataIntellectTech kdbx-modules · Discussion #20
Project Owner
Jonathon McMurray