deprecate pkg/parsers.ParseKeyValueOpt and move internal #49177
Conversation
thaJeztah
added
status/2-code-review
kind/refactor
thaJeztah
changed the title
thaJeztah
added
impact/deprecation
area/go-sdk
thaJeztah
force-pushed
the
daemon_no_pkg_parsers
branch
from
18c1d16 to
35eeb00
Compare
| valids := map[string][]string{ | ||
| "key=value": {"key", "value"}, | ||
| " key = value ": {"key", "value"}, | ||
| "key=value1=value2": {"key", "value1=value2"}, | ||
| " key = value1 = value2 ": {"key", "value1 = value2"}, | ||
| } |
There was a problem hiding this comment.
In all honesty, I'm even considering removing this utility altogether, and use straight strings.Cut.
The only added value of this utility is to allow for badly formatted values that have spaces around the <key> = <value>. We don't document those as supported (but admittedly, documentation is sparse, except for some examples).
Removing the utility could break some cases where options are incorrectly formatted, but likely mostly corner-cases; perhaps a better option would be to actively invalidate those.
There was a problem hiding this comment.
LOL, and I completely forgot I had a PR for exec-opts, also related to this;
There was a problem hiding this comment.
On the flip side, I don't think it's a big burden to keep it for now. But I'm fine either way.
There was a problem hiding this comment.
Yeah, definitely not a huge burden in this case. It's sometimes the ripple-effect it can have.
In general; we've made some bad choices to be too permissive in some cases. Or at least; too permissive in the wrong place. We tried to address for possible variants to keep a slightly easier UX ("we very likely know what you meant, so let's accept that's what you meant"); that's probably fine for some cases, but for those we should've kept it "UX", normalized on the way in, and kept the API strict.
The downside of some of the choices we made is that we kept poorly formatted values; that seemed like a small burden, but also means that every place handling those values must account for them to be potentially poorly formatted;
- compare case-insensitive
- trim spaces (possibly multiple times)
- in some cases, we could even have issues detecting duplicate options where no duplicates are allowed, but because they're formatted differently, we need to do the extra handling to parse before we can detect if it's a duplicate
So, yeah, where possible, I'm trying to find ways out of those situations (handling of "non-normalized" (ubuntu) vs "normalized" (docker.io/.library/ubuntu:latest) image references is another good example of that).
thaJeztah
force-pushed
the
daemon_no_pkg_parsers
branch
from
35eeb00 to
f0c0c13
Compare
thaJeztah
force-pushed
the
daemon_no_pkg_parsers
branch
from
f0c0c13 to
40ce0c9
Compare
thaJeztah
force-pushed
the
daemon_no_pkg_parsers
branch
from
40ce0c9 to
9e2aedb
Compare
Move the utility to where it's used, and deprecate the implementation in pkg/parsers. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
force-pushed
the
daemon_no_pkg_parsers
branch
from
9e2aedb to
5b18a79
Compare
|
Rebased (merge conflict in imports only) |
/pkg#32989deprecate pkg/parsers.ParseKeyValueOpt and move internal
Move the utility to where it's used, and deprecate the implementation
in pkg/parsers.
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)