Repository navigation
fix(utils): reject mi, ni and ui as quantity suffixes - #2748
Conversation
kubernetes-prow
Bot
added
release-note
|
|
kubernetes-prow
Bot
added
cncf-cla: no
694d9a8 to
c831a6d
Compare
|
/easycla |
c831a6d to
b976cf7
Compare
|
/easycla |
kubernetes-prow
Bot
added
cncf-cla: yes
| # handle SI inconsistency | ||
| if suffix == "ki": | ||
| # handle SI inconsistency: "ki", "mi", "ni" and "ui" are not valid binary suffixes | ||
| if base == 1024 and suffix[0] not in _BINARY_PREFIXES: |
There was a problem hiding this comment.
do you need the conditiion: base == 1024 ?
There was a problem hiding this comment.
Good question. Yes, it was needed: the check applies only to the binary (i) suffixes, and without the base == 1024 guard the valid base-1000 suffixes m, n, u and k would be rejected too, since they are not in KMGTPE.
That it reads as redundant is a fair point though, so I moved the check into the endswith("i") branch in both parse_quantity and format_quantity. The base == 1024 condition is gone, behaviour is the same, and the existing tests pass.
parse_quantity and format_quantity only rejected 'ki' among the binary
suffixes, so 'mi', 'ni' and 'ui' were accepted as base-1024 negative
exponents (e.g. parse_quantity('1mi') == 1/1024). Binary suffixes are limited
to Ki, Mi, Gi, Ti, Pi and Ei.
b976cf7 to
ed38fb4
Compare
|
/lgtm |
kubernetes-prow
Bot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ArfiArfath21, yliaog The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kubernetes-prow
Bot
added
the
approved
f65a97e
into
kubernetes-client:master
2 participants
What type of PR is this?
/kind bug
What this PR does / why we need it:
parse_quantityandformat_quantityonly rejectedkiamong the binary suffixes, somi,nianduiwere accepted as base-1024 negative exponents (parse_quantity("1mi")returned1/1024,format_quantity(Decimal(1), "mi")returned"1024mi"). Binary suffixes are now limited toKi,Mi,Gi,Ti,PiandEi, as in apimachinery'squantity.go. A typo such as512minow raisesValueErrorinstead of silently becoming a tiny number.Which issue(s) this PR fixes:
Fixes #2746
Special notes for your reviewer:
Tests added to
TestUtilsUnitTestsinkubernetes/e2e_test/test_utils.py; they fail onmasterand pass with this change.AI assistance disclosure: prepared with Claude Code; I reviewed the change and ran the tests locally.
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: