[GitHub] maskit commented on issue #721: Parse authParamsString on plugin side

[gi...@git.apache.org]

maskit commented on issue #721: Parse authParamsString on plugin side
URL: https://github.com/apache/incubator-pulsar/pull/721#issuecomment-326971888
 
 
   @tkb77 Can you explain why we should parse JSON in `AuthenticationFactory` in spite of the fact that plugins can easily do that in the new `configure`? It would be just a line.
   ```java
   Map<String, String> authParams = AuthenticationUtil.configureFromJsonString(authParamsStr);
   ```
   I don't see pros keeping the old `configure` interface and supporting new format in it. I think it should be left just for compatibility.
   
   Also, format auto-detecting is not so easy.
   ```
   "abc":"x,y,z"}
   ```
   Note that `{` is missing. This string would be parsed as a colon-comma format string, which means that `abc` would not be set without any errors. This is unsafe.
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services