-
Notifications
You must be signed in to change notification settings - Fork 3
Add usage lib #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
jonathonmcmurray
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a few style tweaks to bring this in line with other packages please
|
@jmlemay could we pull out the unit test framework parts of this package and merge them separately to the main code base so that we all the packages can use the same framework code please? |
jonnypress
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good generally as first version.
Question (which we can follow up in second pass). Does it handle very long input query strings, i.e. greater than whatever \c is set to? In the TorQ version they get truncated in the log file. I don't know a good solution but I guess there probably is one (there are pros and cons to the truncation).
jonnypress
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved (sorry meant to hit approve on the last update)
It handles long query strings in that it will execute them, but they will be truncated in the logs to whatever \c is. E.g. I suppose the best solution is probably to make this configurable? Could be done in a second version. At the moment it's configurable simply by setting \c, with the only limitation being the max console size. |
Same concept as TorQ's logusage script.
Basic by-hand testing for kdb+ 3.0 and above on Linux