[ 
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]

Reply via email to