"consul" label naming case sensitivity

[prein]

Hi. It took me some time to figure out that "consul" label name was case sensitive. All other labels I use are uppercase, so this one stands out now. Why not make it case insensitive? Another suggestion regarding the naming is to make it more descriptive and specific. Perhaps something along the lines of "consul_custom_service_name" - long but descriptive, at least for me. Thanks

[janisz]

HI @prein

Why not make it case insensitive?

I checked and consul in case insensitive when querying for the service, but it store service name with case. I'm not sure if we should make service name upper or lowercase if user want to have mixed case. Maybe this could be a config option to ignore case and keep everything lowercase.

Another suggestion regarding the naming is to make it more descriptive and specific.

I concur but it won't be backward compatible. The history behind "consul" flag: Initially we thought it will be used only to distinguish tasks that should be handled by marathon-consul, so the name consul was good. Then we added functionality to store service name there and that was a mistake. Short label name was useful from Marathon UI perspective but as you pointed out not clear for user.

Your points are perfectly valid but I afraid we can't fix this without breaking backward compatibility. What do you think? Maybe we can do better job on documentation?

[prein]

Thanks @janisz. I understand and agree with your concern about backward compatibility. Perhaps adding additional label and making the code ignore the "old" one when the "new" one is found would be good for transitional stage. Making it clear in documentation is also good idea. Perhaps I wouldn't spend time guessing if it was pointed out there.

[janisz]

Sounds good. We can make it working like this.

@adamdubiel What do you think about this? @prein Would you like to prepare a patch for it?

[adamdubiel]

I think we could make the name of the label configurable? This way anyone can have it his or hers way (upper case, more descriptive etc). Also it could be a list, so this way we make it easy to transition for one state to another.

However since we are not in immediate need of this feature, i would say this qualifies as best effort type of task and as @janisz proposed, creating a PR would speed up things significantly. Of course we will take care of reviewing & releasing new version once it is merged.