Hello Sandro Bonazzola,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/13472
to review the following change.
Change subject: tools: fixed --version output
......................................................................
tools: fixed --version output
--version previously showed 1.0.0. Now it shows the package name and
version as set in configure.ac.
--local-tmp default value is now randomly generated using mkdtemp.
The directory is cleaned up if -h, --version or --help where supplied
avoiding to leave unwanted empty directories around.
Moved some other constants to config.py using configure to set them.
Change-Id: I88f53765028284221a0506efbc595b146a2bca8a
Bug-Url: https://bugzilla.redhat.com/886052
Signed-off-by: Sandro Bonazzola <[email protected]>
---
A build/subst.inc
M configure.ac
M src/Makefile.am
M src/__main__.py
A src/config.py.in.in
5 files changed, 94 insertions(+), 20 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-log-collector
refs/changes/72/13472/1
diff --git a/build/subst.inc b/build/subst.inc
new file mode 100644
index 0000000..1c414f9
--- /dev/null
+++ b/build/subst.inc
@@ -0,0 +1,30 @@
+#
+# ovirt-log-collector
+# Copyright (C) 2012-2013 Red Hat, Inc.
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+
+.in:
+ if sed \
+ -e 's#[@]engineconfigdir_POST@#$(engineconfigdir)#g' \
+ -e 's#[@]sysconfdir_POST@#$(sysconfdir)#g' \
+ -e 's#[@]localstatedir_POST@#$(localstatedir)#g' \
+ $< > [email protected]; then \
+ mv "[email protected]" "$@"; \
+ else \
+ rm -f "[email protected]"; \
+ false; \
+ fi
diff --git a/configure.ac b/configure.ac
index 2deb375..63039f5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -76,6 +76,7 @@
AC_CONFIG_FILES([
Makefile
ovirt-log-collector.spec
+ src/config.py.in
src/Makefile
src/sos/Makefile
src/sos/plugins/Makefile
diff --git a/src/Makefile.am b/src/Makefile.am
index b574545..5ace643 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -16,13 +16,28 @@
#
include $(top_srcdir)/build/python.inc
+include $(top_srcdir)/build/subst.inc
MAINTAINERCLEANFILES = \
$(srcdir)/Makefile.in \
$(NULL)
+SUFFIXES = .in
+
+CLEANFILES = \
+ config.py \
+ $(NULL)
+
+config.py: \
+ $(top_srcdir)/configure.ac \
+ $(NULL)
+
SUBDIRS = sos helper
+ovirtlogcollectorlib_PYTHON = \
+ config.py\
+ $(NULL)
+
dist_ovirtlogcollectorlib_PYTHON = \
__init__.py \
__main__.py \
diff --git a/src/__main__.py b/src/__main__.py
index 8717de0..d7cae54 100755
--- a/src/__main__.py
+++ b/src/__main__.py
@@ -31,23 +31,21 @@
import datetime
import dateutil.parser
import dateutil.tz as tz
+import tempfile
+import atexit
from helper import hypervisors
+from ovirt_log_collector import config
-versionNum = "1.0.0"
STREAM_LOG_FORMAT = '%(levelname)s: %(message)s'
FILE_LOG_FORMAT = \
'%(asctime)s::%(levelname)s::%(module)s::%(lineno)d::%(name)s:: \
%(message)s'
FILE_LOG_DSTMP = '%Y-%m-%d %H:%M:%S'
-DEFAULT_SSH_KEY = "/etc/pki/ovirt-engine/keys/engine_id_rsa"
DEFAULT_SSH_USER = 'root'
-DEFAULT_CONFIGURATION_FILE = "/etc/ovirt-engine/logcollector.conf"
-DEFAULT_SCRATCH_DIR = '/tmp/logcollector'
-DEFAULT_LOG_FILE = '/var/log/ovirt-engine/engine-log-collector.log'
DEFAULT_TIME_SHIFT_FILE = 'time_diff.txt'
-FILE_PG_PASS = "/etc/ovirt-engine/.pgpass"
PGPASS_FILE_ADMIN_LINE = "DB ADMIN credentials"
+DEFAULT_SCRATCH_DIR = None # Will be initialized by __main__
# Default DB connection params
pg_user = 'postgres'
@@ -71,11 +69,11 @@
"Error: unknown value type '%s' was requested" % dbconf_param
)
inDbAdminSection = False
- if (os.path.exists(FILE_PG_PASS)):
+ if (os.path.exists(config.FILE_PG_PASS)):
logging.debug(
"Found existing pgpass file, fetching DB %s value" % dbconf_param
)
- with open(FILE_PG_PASS) as pgPassFile:
+ with open(config.FILE_PG_PASS) as pgPassFile:
for line in pgPassFile:
# find the line with "DB ADMIN"
@@ -254,8 +252,8 @@
File=(%s)" % self.options.conf_file
)
- elif os.path.isfile(DEFAULT_CONFIGURATION_FILE):
- self.from_file(DEFAULT_CONFIGURATION_FILE)
+ elif os.path.isfile(config.DEFAULT_CONFIGURATION_FILE):
+ self.from_file(config.DEFAULT_CONFIGURATION_FILE)
def from_option_groups(self, options, parser):
for optGrp in parser.option_groups:
@@ -1051,6 +1049,17 @@
setup_pg_defaults()
+ DEFAULT_SCRATCH_DIR = tempfile.mkdtemp(prefix='logcollector-')
+
+ commandline = set(sys.argv)
+ cleanup_set = set(["--help", "-h", "--version"])
+
+ def cleanup():
+ os.rmdir(DEFAULT_SCRATCH_DIR)
+
+ if len(commandline.intersection(cleanup_set)) != 0:
+ atexit.register(cleanup)
+
def comma_separated_list(option, opt_str, value, parser):
setattr(
parser.values, option.dest, [v.strip() for v in value.split(",")]
@@ -1070,14 +1079,17 @@
OptionParser.format_epilog = lambda self, formatter: self.epilog
parser = OptionParser(
usage_string,
- version="Version " + versionNum,
+ version="{pkg_name}-{pkg_version}".format(
+ pkg_name=config.PACKAGE_NAME,
+ pkg_version=config.PACKAGE_VERSION
+ ),
epilog=epilog_string
)
parser.add_option(
"", "--conf-file", dest="conf_file",
help="path to configuration file (default=%s)" % (
- DEFAULT_CONFIGURATION_FILE
+ config.DEFAULT_CONFIGURATION_FILE
),
metavar="PATH"
)
@@ -1085,7 +1097,7 @@
parser.add_option(
"", "--local-tmp", dest="local_tmp_dir",
help="directory to copy reports to locally \
-(default=%s)" % DEFAULT_SCRATCH_DIR,
+(default is randomly generated like: %s)" % DEFAULT_SCRATCH_DIR,
metavar="PATH",
default=DEFAULT_SCRATCH_DIR
)
@@ -1112,17 +1124,19 @@
parser.add_option(
"", "--log-file",
dest="log_file",
- help="path to log file (default=%s)" % DEFAULT_LOG_FILE,
+ help="path to log file (default=%s)" % (
+ config.DEFAULT_LOG_FILE
+ ),
metavar="PATH",
- default=DEFAULT_LOG_FILE
+ default=config.DEFAULT_LOG_FILE
)
parser.add_option(
"", "--cert-file", dest="cert_file",
help="The CA certificate used to validate the engine. \
-(default=/etc/pki/ovirt-engine/ca.pem)",
- metavar="/etc/pki/ovirt-engine/ca.pem",
- default="/etc/pki/ovirt-engine/ca.pem"
+(default=%s)" % config.DEFAULT_CA_PEM,
+ metavar=config.DEFAULT_CA_PEM,
+ default=config.DEFAULT_CA_PEM
)
parser.add_option(
@@ -1223,9 +1237,9 @@
If a identity file is not supplied the program will prompt for a password.
It is strongly recommended to use key based authentication with SSH because
the program may make multiple SSH connections resulting in multiple requests
-for the SSH password.""" % DEFAULT_SSH_KEY,
+for the SSH password.""" % config.DEFAULT_SSH_KEY,
metavar="KEYFILE",
- default=DEFAULT_SSH_KEY
+ default=config.DEFAULT_SSH_KEY
)
ssh_group.add_option(
diff --git a/src/config.py.in.in b/src/config.py.in.in
new file mode 100644
index 0000000..39a1704
--- /dev/null
+++ b/src/config.py.in.in
@@ -0,0 +1,14 @@
+"""
+Paths and version constants for @PACKAGE_NAME@
+"""
+
+PACKAGE_NAME = "@PACKAGE_NAME@"
+PACKAGE_VERSION = "@PACKAGE_VERSION@"
+
+FILE_PG_PASS = "@engineconfigdir_POST@/.pgpass"
+
+DEFAULT_CA_PEM = "@sysconfdir_POST@/pki/ovirt-engine/ca.pem"
+DEFAULT_SSH_KEY = "@sysconfdir_POST@/pki/ovirt-engine/keys/engine_id_rsa"
+DEFAULT_CONFIGURATION_FILE = "@engineconfigdir_POST@/logcollector.conf"
+DEFAULT_LOG_FILE = \
+ "@localstatedir_POST@/log/ovirt-engine/engine-log-collector.log"
--
To view, visit http://gerrit.ovirt.org/13472
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I88f53765028284221a0506efbc595b146a2bca8a
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-log-collector
Gerrit-Branch: ovirt-log-collector-3.2
Gerrit-Owner: Kiril Nesenko <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
