8:54 p.m.
Commit 4a7fae9 changed the revision returned by get_provider_version
to a string. However, the revision has to be a number because (since
git is used) it's basically the number of commits in HEAD.
Comparing a string against an int in Python 2.x will always result in
the string value being greater than the int value. Since there
are a lot of compares against revision numbers all over the test
scripts, this change is probably the most economic.
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
Not being a python guy I got wacked myself when I was testing
my (soon to come) cimtest changes against a pre-console libvirt-cim.
'1272' > 1276 silently returned True leading to epic failures :-(
suites/libvirt-cim/lib/XenKvmLib/const.py | 4 +++-
suites/libvirt-cim/lib/XenKvmLib/reporting.py | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/suites/libvirt-cim/lib/XenKvmLib/const.py
b/suites/libvirt-cim/lib/XenKvmLib/const.py
index a8d0254..a454b2f 100755
--- a/suites/libvirt-cim/lib/XenKvmLib/const.py
+++ b/suites/libvirt-cim/lib/XenKvmLib/const.py
@@ -179,7 +179,9 @@ def get_provider_version(virt, ip):
revision = revision.strip("+")
if revision.isdigit():
revision = int(revision)
+ else:
+ raise Exception("revision %s is not a digit", revision)
- return str(revision), str(changeset)
+ return revision, changeset
diff --git a/suites/libvirt-cim/lib/XenKvmLib/reporting.py
b/suites/libvirt-cim/lib/XenKvmLib/reporting.py
index 67ec974..88375b0 100644
--- a/suites/libvirt-cim/lib/XenKvmLib/reporting.py
+++ b/suites/libvirt-cim/lib/XenKvmLib/reporting.py
@@ -101,7 +101,7 @@ def get_env_data(ip, virt):
rev, changeset = get_provider_version(virt, ip)
cimtest_revision, cimtest_changeset = get_cimtest_version()
- lc_ver = "Libvirt-cim revision: %s\nLibvirt-cim changeset: %s\n" % \
+ lc_ver = "Libvirt-cim revision: %d\nLibvirt-cim changeset: %s\n" % \
(rev, changeset)
cimtest_ver = "Cimtest revision: %s\nCimtest changeset: %s\n" % \
(cimtest_revision, cimtest_changeset)
--
1.7.9.5
9:46 p.m.
On 09/17/2013 08:54 AM, Viktor Mihajlovski wrote:
Commit 4a7fae9 changed the revision returned by get_provider_version
to a string. However, the revision has to be a number because (since
git is used) it's basically the number of commits in HEAD.
Comparing a string against an int in Python 2.x will always result in
the string value being greater than the int value. Since there
are a lot of compares against revision numbers all over the test
scripts, this change is probably the most economic.
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
---
Not being a python guy I got wacked myself when I was testing
my (soon to come) cimtest changes against a pre-console libvirt-cim.
'1272' > 1276 silently returned True leading to epic failures :-(
suites/libvirt-cim/lib/XenKvmLib/const.py | 4 +++-
suites/libvirt-cim/lib/XenKvmLib/reporting.py | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)
I guess this is a mea culpa and me too moment all wrapped into one.
Python isn't necessarily my strength and I probably should have looked
more deeply into all the callers. I guess I was too hyperfocused on the
error I got that resulted in the bad changes I made...
diff --git a/suites/libvirt-cim/lib/XenKvmLib/const.py
b/suites/libvirt-cim/lib/XenKvmLib/const.py
index a8d0254..a454b2f 100755
--- a/suites/libvirt-cim/lib/XenKvmLib/const.py
+++ b/suites/libvirt-cim/lib/XenKvmLib/const.py
@@ -179,7 +179,9 @@ def get_provider_version(virt, ip):
revision = revision.strip("+")
if revision.isdigit():
revision = int(revision)
+ else:
+ raise Exception("revision %s is not a digit", revision)
- return str(revision), str(changeset)
+ return revision, changeset
diff --git a/suites/libvirt-cim/lib/XenKvmLib/reporting.py
b/suites/libvirt-cim/lib/XenKvmLib/reporting.py
index 67ec974..88375b0 100644
--- a/suites/libvirt-cim/lib/XenKvmLib/reporting.py
+++ b/suites/libvirt-cim/lib/XenKvmLib/reporting.py
@@ -101,7 +101,7 @@ def get_env_data(ip, virt):
rev, changeset = get_provider_version(virt, ip)
cimtest_revision, cimtest_changeset = get_cimtest_version()
- lc_ver = "Libvirt-cim revision: %s\nLibvirt-cim changeset: %s\n" % \
+ lc_ver = "Libvirt-cim revision: %d\nLibvirt-cim changeset: %s\n" % \
Still not quite right - 'changeset' is more of a hexadecimal value, but
printing as such (either %x or %d) would result in a traceback and the
error message "a number is required, not unicode".
If I print 'Revision' and 'Changeset' in 'get_provider_version'
right
after they are fetched from 'inst', I get:
1262
eb776e2
To "fix" I changed the "(rev, changeset)" to "(rev,
str(changeset))"
I adjusted and pushed.
Of course I did a bit more digging just to be sure...
'changeset' is ever compared against is 'sles11_changeset'
It's initially generated in the libvirt-cim build as a "'git rev-parse
--short HEAD' > .changeset" by autoconfiscate.sh. If unavailable it's
set to 'Unknown' in the configure script as part of a build environment
variable "LIBVIRT_CIM_CS".
While at it - I checked the generation of "revision" too. In this case,
it's the result of the number of commits from a "git rev-list HEAD | wc
-l > .revision". It's then saved/used as the build environment variable
"LIBVIRT_CIM_RV". In my example, it shows as 1262, although to be
technically correct it should be 1279. If it's not possible to get the
value it defaults to '0' (zero). So printing/using as an 'int' is
certainly the right thing to do.
I'm not a "build guru", but I think the libvirt-cim build probably needs
to generate the values more frequently than just in autoconfiscate.sh
seeing as that's 'perhaps' run less frequently than someone making
changes to the git repository and rebuilding...
John
(rev, changeset)
cimtest_ver = "Cimtest revision: %s\nCimtest changeset: %s\n" % \
(cimtest_revision, cimtest_changeset)
11:12 p.m.
On 10/02/2013 03:46 PM, John Ferlan wrote:
...
To "fix" I changed the "(rev, changeset)" to "(rev,
str(changeset))"
I adjusted and pushed.
thanks
...
I'm not a "build guru", but I think the libvirt-cim build probably needs
to generate the values more frequently than just in autoconfiscate.sh
seeing as that's 'perhaps' run less frequently than someone making
changes to the git repository and rebuilding...
I agree it is a bit tricky in
development environments, in the
long run however I think it's correct to calculate the 'official'
revision only during full (RPM) builds from GIT master.
The problem is that everyone is typically working from private
branches which tend to be 'longer' than master.
A section in a cimtest README explaining the ramifications
might help though...
--
Mit freundlichen Grüßen/Kind Regards
Viktor Mihajlovski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
4050
days inactive
4065
days old
2 comments
2 participants
participants (2)
-
John Ferlan -
Viktor Mihajlovski