[jira] [Comment Edited] (MESOS-3781) Replace Master/Slave Terminology Phase I - Add duplicate agent flags

["Jay Guo (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/MESOS-3781?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15245223#comment-15245223 ] 

Jay Guo edited comment on MESOS-3781 at 4/18/16 7:10 AM:
---------------------------------------------------------

Here's what I understand from your comments:
1. We should enable multi-named flags in FlagsBase
2. While loading flag values from cmd/env in FlagsBase::load(), it generates warnings by determining actual name being used. (Add check logic to __Flag.load__ lambda? It takes __DeprecatedName__ struct in capture, as well as the _name_ used to actual load the value, and generate warnings if _name_ falls into DeprecatedName) Something like this:
{code}
flag.load = [t1, deprecatedNames](FlagsBase*, const std::string& name, const std::string& value) -> Try<Nothing> {
  ...
  if (deprecatedNames.find(name)) { deprecationWarning(name); }
  ...
};
{code}
3. Add duplicate names to all applicable flags

My concerns:
1. Why both _Name_ and _deprecatedName_ structs? Since we only need to know whether it is deprecated. Also, I don't see any instance that has __multiple__ deprecated names, so why vector of structs?
2. If the sole purpose of having this vector of structs is to search for deprecated names, I suggest to use _set_ instead.
3. Are we overengineering this? 'slave' flags will eventually be removed, along with deprecatedNames. Nevertheless, I like the idea of having multi-name flags.

Thanks!


was (Author: guoger):
Here's what I understand from your comments:
1. We should enable multi-named flags in FlagsBase
2. While loading flag values from cmd/env in FlagsBase::load(), it generates warnings by determining actual name being used. (Add check logic to __Flag.load__ lambda? It takes __DeprecatedName__ struct in capture, as well as the _name_ used to actual load the value, and generate warnings if _name_ falls into DeprecatedName) Something like this:
```cpp
flag.load = [t1, deprecatedNames](FlagsBase*, const std::string& name, const std::string& value) -> Try<Nothing> {
  ...
  if (deprecatedNames.find(name)) { deprecationWarning(name); }
  ...
};
```
3. Add duplicate names to all applicable flags

My concerns:
1. Why both _Name_ and _deprecatedName_ structs? Since we only need to know whether it is deprecated. Also, I don't see any instance that has __multiple__ deprecated names, so why vector of structs?
2. If the sole purpose of having this vector of structs is to search for deprecated names, I suggest to use _set_ instead.
3. Are we overengineering this? 'slave' flags will eventually be removed, along with deprecatedNames. Nevertheless, I like the idea of having multi-name flags.

Thanks!

> Replace Master/Slave Terminology Phase I - Add duplicate agent flags 
> ---------------------------------------------------------------------
>
>                 Key: MESOS-3781
>                 URL: https://issues.apache.org/jira/browse/MESOS-3781
>             Project: Mesos
>          Issue Type: Task
>            Reporter: Diana Arroyo
>            Assignee: Jay Guo
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)