Hi Ken,
----- Original Message -----
> On 18/01/16 21:00, Ken McDonell wrote:
> > ...
> > kenj@bozo:~/src/pcp/qa$ pminfo -c /tmp/eek foo
> > [/tmp/eek:1] Error: pmRegisterDerived(foo, ...) syntax error
> > rat(disk.dev.read)
> > ^
> > Error: foo: Unknown metric name
> > kenj@bozo:~/src/pcp/qa$ cat /tmp/eek
> > foo = rat(disk.dev.read)
>
> This is NOT a Python wrapper issue and we do NOT need a libpcp API change.
There's a python wrapper improvement needed here still I think, and
while libpcp doesn't *have* to change to help, it would help alot.
The current convention of requiring the script to know the C quirk
(and this libpcp interface differs to many other libpcp interfaces,
oddly) of building an error string from expression and offset, and
then also extracting an additional diagnostic ... is missing. This
should be handled in the wrapper (in python code, or via a libpcp
helper), so that scripts don't have to do this manually.
The current derived metrics code error handling (even with fixes to
not return null) is inconsistent with the rest of libpcp, which is
confusing and has led to this bug - we should fix that.
man pmparsectime pmparsehostattrsspec pmparsehostspec pmparsetime \
pmparsemetricspec pmparseinterval pmparsetimewindow pmparseunitsstr
and compare to the derived metrics parser error handling ... one of
these things is not like the others.
> The C code appears to get it "right" because it does not call
> pmRegisterDerived(), it calls pmLoadDerivedConfig() ... the error
> message from libpcp contains the wrong function name above.
That'll be because pmLoadDerivedConfig calls pmRegisterDerived
internally though - derive.c around line 1668...
errp = pmRegisterDerived(q, ep);
if (errp != NULL) {
pmprintf("[%s:%d] Error: pmRegisterDerived(%s, ...) syntax
error\n", fname, lineno, q);
pmprintf("%s\n", &buf[eq+1]);
So, I think that diagnostic is OK.
> So returning to Marko's original questions ...
>
> > Here pmDerivedErrStr returns nothing. Is this expected or should
> > pmDerivedErrStr be improved to return always something understandable
> > in case of errors?
>
> To which the answers are NO and YES!
*nod*.
> kenj@bozo:~/tmp$ bad.py
> PMAPI exception as requested
> bad.py: Generic error, already reported above ['@', '(disk.dev.read']
This python error handling is suboptimal, it'd be good to have the string
built consistently; other direct callers of pmRegisterDerived will be in
the same boat as python, wanting the normal error handling model.
> So I think the way forward is a closer audit of the derived metrics
> parser, not libpcp PMAPI changes.
Hmmm, I suspect we should do both, based on the way all the other libpcp
parser interfaces do error handling.
cheers.
--
Nathan
|