Kuantifier Release Candidate Bugfixes #186
Conversation
kubernetes/kubernetes_meter.py
Outdated
| def getint_roundup(self, key): | ||
| return int(float(self.apel_dict.get(key, 0)) + 0.5) | ||
|
|
There was a problem hiding this comment.
Does this actually work?
blin@blin-work:~$ python
Python 3.12.3 (main, Apr 17 2024, 00:00:00) [GCC 13.2.1 20240316 (Red Hat 13.2.1-7)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> int(float(3.9))
3
Are you looking for math.ceiling() instead?
There was a problem hiding this comment.
This rounds >= X.5 to X +1, <= X.5 to X, eg:
>>> int(float("3.9") + .5)
4
>>> int(float("3.4") + .5)
3
. math.ceiling() is probably more appropriate though
There was a problem hiding this comment.
Ah duh, that makes sense. math.ceiling doesn't do any sort of rounding so maybe what you had was right?
I'm not actually sure I understand what the ultimate goal is here.
There was a problem hiding this comment.
CPU requests are configured as 0.9 and 7.9 for 1 and 8 core jobs respectively in Armen's cluster to allow other small pods to run on the same node, which were getting truncated to 0 or 7 by this code. Either math.ceil or the +.5 trick will round them to 1 and 8 as desired in this case. I think math.ceil is slightly preferable since it will round a processor count of <0.5 to 1 instead of 0, but that is probably a rare corner case.
There was a problem hiding this comment.
Huh and why do we need to turn this into an int?
There was a problem hiding this comment.
That is a good question. I looked at several sample gratia records while implementing this and noticed that all fields were integers, but never confirmed that that was actually a requirement
kubernetes/kubernetes_meter.py
Outdated
| # Handle case where apel values contain colons (eg. 'SubmitHost: http://localhost:8000') | ||
| kv_pair_match = re.match(r'([A-Za-z0-9_]+)\s*:\s*([^\s].*[^\s])\s*',line) |
There was a problem hiding this comment.
How about just limiting the number of splits?
>>> 'SubmitHost: http://localhost:8000'.split(':', maxsplit=1)
['SubmitHost', ' http://localhost:8000']
There was a problem hiding this comment.
Didn't know about the maxsplit kwarg, that's very useful
Noneenvfile argument, which is the default:is in the value (eg.SubmitHost: http://localhost:8000)