pcp
[Top] [All Lists]

pmrep: fix command line parsing

To: pcp developers <pcp@xxxxxxxxxxx>
Subject: pmrep: fix command line parsing
From: Marko Myllynen <myllynen@xxxxxxxxxx>
Date: Sun, 13 Dec 2015 17:49:51 +0200
Delivered-to: pcp@xxxxxxxxxxx
Organization: Red Hat
Reply-to: myllynen@xxxxxxxxxx
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0
Hi,

ef4312 tried to address the issue of not being able to provide options
after metrics but it turned out that that route hit the wall elsewhere
(RHBZ#1289912) and it also caused a horrible regression where command
line options did not override configuration file parameters any more.

Revert back to what we had earlier in use for a long time and sort
out things in a more robust fashion after the release as/if needed.

To test, try e.g. the vmstat example from pmrep.conf with -E 5.

(Tweak few help texts to fit in one line while at it.)

---
 src/pmrep/TODO     |  1 +
 src/pmrep/pmrep.1  |  2 ++
 src/pmrep/pmrep.py | 24 +++++++++++++++++-------
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/pmrep/TODO b/src/pmrep/TODO
index e25d255..7682749 100644
--- a/src/pmrep/TODO
+++ b/src/pmrep/TODO
@@ -1,4 +1,5 @@
 -  after first release: drop all compat code
+-  after first release: check/fix get_cmd_line_metrics for good
 -  after first release: check all code marked with BZ references
 -  allow defining instances to display [-i]
 -  opt to write cols per row with stdout output [-k]
diff --git a/src/pmrep/pmrep.1 b/src/pmrep/pmrep.1
index 653561b..a1ab7bc 100644
--- a/src/pmrep/pmrep.1
+++ b/src/pmrep/pmrep.1
@@ -676,6 +676,8 @@ $ pmrep -o archive -F ./a -t 5s -W 5m ds389 xfs 
kernel.all.cpu disk mem
 Default configuration file.
 .PD
 .SH BUGS
+No command line option can follow metrics.
+
 Specifying instances is not yet supported.
 .SH "PCP ENVIRONMENT"
 Environment variables with the prefix
diff --git a/src/pmrep/pmrep.py b/src/pmrep/pmrep.py
index db90ea9..eaed656 100644
--- a/src/pmrep/pmrep.py
+++ b/src/pmrep/pmrep.py
@@ -175,6 +175,7 @@ class PMReporter(object):
                      'zabbix_server', 'zabbix_port', 'zabbix_host', 
'zabbix_interval')
 
         # Special command line switches
+        self.argless = ('-C', '--check', '-L', '--local-PMDA', '-H', 
'--no-header', '-U', '--no-unit-info', '-G', '--no-globals', '-p', 
'--timestamps', '-d', '--delay', '-r', '--raw', '-x', '--extended-header', 
'-u', '--no-interpol', '-z', '--hostzone')
         self.arghelp = ('-?', '--help', '-V', '--version')
 
         # The order of preference for parameters (as present):
@@ -316,7 +317,7 @@ class PMReporter(object):
         opts.pmSetLongOptionSpecLocal()    # -K/--spec-local
         opts.pmSetLongOption("config", 1, "c", "FILE", "config file path")
         opts.pmSetLongOption("check", 0, "C", "", "check config and metrics 
and exit")
-        opts.pmSetLongOption("output", 1, "o", "OUTPUT", "output target, one 
of: archive, csv, stdout (default), zabbix")
+        opts.pmSetLongOption("output", 1, "o", "OUTPUT", "output target: 
archive, csv, stdout (default), or zabbix")
         opts.pmSetLongOption("output-archive", 1, "F", "ARCHIVE", "output 
archive (with -o archive)")
         opts.pmSetLongOption("derived", 1, "e", "FILE|DFNT", "derived metrics 
definitions")
         opts.pmSetLongOptionDebug()        # -D/--debug
@@ -345,7 +346,7 @@ class PMReporter(object):
         opts.pmSetLongOption("extended-header", 0, "x", "", "display extended 
header")
         opts.pmSetLongOption("repeat-header", 1, "E", "N", "repeat stdout 
headers every N lines")
         opts.pmSetLongOption("timestamp-format", 1, "f", "STR", "strftime 
string for timestamp format")
-        opts.pmSetLongOption("no-interpolation", 0, "u", "", "disable 
interpolation mode with archives")
+        opts.pmSetLongOption("no-interpol", 0, "u", "", "disable interpolation 
mode with archives")
         opts.pmSetLongOption("count-scale", 1, "q", "SCALE", "default count 
unit")
         opts.pmSetLongOption("space-scale", 1, "b", "SCALE", "default space 
unit")
         opts.pmSetLongOption("time-scale", 1, "y", "SCALE", "default time 
unit")
@@ -422,10 +423,19 @@ class PMReporter(object):
 
     def get_cmd_line_metrics(self):
         """ Get metric set specifications from the command line """
-        if any(x in self.arghelp for x in sys.argv):
-            return 0
-        pmapi.c_api.pmGetOptionsFromList(sys.argv) # RHBZ#1287778
-        return self.opts.pmNonOptionsFromList(sys.argv)
+        for arg in sys.argv[1:]:
+            if arg in self.arghelp:
+                return 0
+        metrics = []
+        for arg in reversed(sys.argv[1:]):
+            if arg.startswith('-'):
+                if len(metrics):
+                    if arg not in self.argless and '=' not in arg:
+                        del metrics[-1]
+                break
+            metrics.append(arg)
+        metrics.reverse()
+        return metrics
 
     def parse_metric_info(self, metrics, key, value):
         """ Parse metric information """
@@ -1311,7 +1321,7 @@ class PMReporter(object):
 
     def connect(self):
         """ Establish a PMAPI context to archive, host or local, via args """
-        self.context = pmapi.pmContext.fromOptions(self.opts, sys.argv, 
self.opts.pmGetOptionContext())
+        self.context = pmapi.pmContext.fromOptions(self.opts, sys.argv)
 
 if __name__ == '__main__':
     try:

Thanks,

-- 
Marko Myllynen

<Prev in Thread] Current Thread [Next in Thread>
  • pmrep: fix command line parsing, Marko Myllynen <=