[
https://issues.apache.org/jira/browse/YUNIKORN-2268?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Peter Bacsko updated YUNIKORN-2268:
-----------------------------------
Target Version: 1.9.0 (was: 1.8.0)
> property key should be treated as case-sensitive
> ------------------------------------------------
>
> Key: YUNIKORN-2268
> URL: https://issues.apache.org/jira/browse/YUNIKORN-2268
> Project: Apache YuniKorn
> Issue Type: Improvement
> Reporter: Chia-Ping Tsai
> Assignee: Dong-Lin Hsieh
> Priority: Major
>
> *Summary:*
> # document the rules (site)
> # add check for property key (core)
> # add release entry (release)
>
> *Discussion*
>
> *Chia-Ping*
> {quote}
> dear all,there are 2 questions (or ideas) about configs' properties
> 1. should we validate the property (key)? it can produce fast-fail but the
> behavior get changed.
> 2. should property value be case insensitive? it seems only value of
> {{application.sort.policy}} is case sensitive
> thanks, and please feel free to correct me :)
> {quote}
> *Craig*
> {quote}
> We explicitly do not validate properties because they are meant to be
> extensible. For example, there are numerous logging settings for both the
> shim and core, and neither side knows which are allowed y the other. This
> goes for all properties, not just those for logging. We configure the
> admission controller, shim, and core from the same configuration, so we don’t
> want to reject unknown properties.
> Case-sensitivity is a thorny subject. There’s two things — the keys and the
> values. Values absolutely need to be treated as case-sensitive in general
> because it’s not always the case that two values differing only in case
> represent the same thing. File systems for example may be case-sensitive or
> not, and assuming that “FILE” == “file” is dangerous. There is also language
> considerations. Depending on locale, case equivalence rules can differ at
> runtime. This has in many pieces of software historically led to a wide
> variety of bugs that are difficult to diagnose. IMO, configuration should
> always be case-sensitive to avoid these issues. In the case of YuniKorn we
> have some historical baggage around some property names being declared
> case-insensitive, and it would be a breaking change to make them
> case-sensitive moving forward, so that’s something we will have to live with.
> It may seem like converting existing properties to be case-insensitive would
> be safe, but this can also break in subtle ways. Previously functional
> configurations that had mismatched property names might now result in
> different actions. For example:
> A: true
> a: false
> If ‘a’ is case-sensitive it’s clear which value gets used. If this property
> is now changed to be case-insensitive, what value will the property contain?
> What if these entries are located in two different configmaps
> (yunikorn-defaults and yunikorn-configs)?
> TLDR; existing semantics should be preserved to avoid backwards-compatibility
> problems, and any future properties should be treated as case-sensitive to
> avoid locale-related bugs.
> {quote}
> *Chia-Ping*
> {quote}
> the "supported values" from docs only show the lower case values ... so maybe
> we can fix it directly and don't worry about BC ?
> https://yunikorn.apache.org/docs/user_guide/queue_config#preemptionpolicy
> {quote}
> *Craig*
> {quote}
> If we do change those to case sensitive, it’s probably best to do it
> gradually by allowing wrong case but warning for a release or two, and then
> forcing lowercase later. That would at least warn users who are upgrading
> that their configurations need fixing.
> And of course this definitely would need a release notes entry.
> Even in this case I would only change the handling of the key names, not the
> property values. So for example, application.sorting.policy can be “fifo” or
> “FIFO” today and that’s ok. I wouldn’t change that since it’s been documented
> to work that way.
> Same goes for things like log levels, etc. those are parsed by a 3rd party
> library so we accept what they do.
> {quote}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]