Added custom message property #1
Conversation
| @Name(PluginConstants.PROPERTY_NAME_CUSTOM_MESSAGE) | ||
| @Description("Custom message") | ||
| @Nullable | ||
| public String message=PluginConstants.PROPERTY_CONFIG_DEFAULT_MESSAGE; |
There was a problem hiding this comment.
Never set values directly using = in config files, config values are set by platform based on user input on web UI.
| public HelloWorldBatchSourceConfig(){ | ||
| if(message==null || message.isEmpty()) | ||
| { | ||
| message=PluginConstants.PROPERTY_CONFIG_DEFAULT_MESSAGE; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Similar comment here , this can be removed !
| @@ -21,5 +36,6 @@ public void validate(FailureCollector failureCollector) { | |||
| public int getFrequency() { | |||
| return frequency == null ? PluginConstants.PROPERTY_DEFAULT_FREQUENCY : frequency; | |||
| } | |||
| public String getMessage(){ return (message == null || message.isEmpty()) ? PluginConstants.PROPERTY_CONFIG_DEFAULT_MESSAGE: message; } | |||
| @@ -5,8 +5,10 @@ | |||
| public final class PluginConstants { | |||
| public static final String PLUGIN_NAME = "HelloWorld"; | |||
| public static final String PROPERTY_NAME_FREQUENCY = "frequency"; | |||
| public static final String PROPERTY_NAME_CUSTOM_MESSAGE="Custom Message"; | |||
There was a problem hiding this comment.
Variable name can be better, this is too generic !
| public static final String PROPERTY_CONFIG_FREQUENCY = "cdap.hello.world.config.frequency"; | ||
| public static final int PROPERTY_DEFAULT_FREQUENCY = 1; | ||
| public static final String PROPERTY_CONFIG_DEFAULT_MESSAGE = "Hello world, this is a custom message"; |
There was a problem hiding this comment.
keep the default message same as before for backwards compatibility!
| @@ -13,6 +13,7 @@ public class HelloWorldInputFormatProvider implements InputFormatProvider { | |||
| public HelloWorldInputFormatProvider(HelloWorldBatchSourceConfig config) { | |||
| configMap = new HashMap<>(); | |||
| configMap.put(PluginConstants.PROPERTY_CONFIG_FREQUENCY, Integer.toString(config.getFrequency())); | |||
| configMap.put(PluginConstants.PROPERTY_CONFIG_DEFAULT_MESSAGE, config.getMessage()); | |||
There was a problem hiding this comment.
This should be a key, although not required it's best to follow the standard, eg : cdap.hello.world.config.frequency
There was a problem hiding this comment.
This one can be cdap.hello.world.config.message
|
Re-base with main and resolve conflicts ! |
|
Resolve merge conflicts and you are missing changes in your |
|
Also lets add a validation the message should not contain |
| @Name(PluginConstants.PROPERTY_NAME_USER_MESSAGE) | ||
| @Description("Custom message") | ||
| @Nullable | ||
| public String message; |
There was a problem hiding this comment.
let's keep these consistent, userMessage is more appropriate.
| configMap.put(PluginConstants.PROPERTY_CONFIG_FREQUENCY, Integer.toString(config.getFrequency())); | ||
|
|
||
| configMap.put(PluginConstants.PROPERTY_CONFIG_KEY_FREQUENCY, Integer.toString(config.getFrequency())); | ||
| configMap.put(PluginConstants.PROPERTY_CONFIG_KEY_MESSAGE, config.getMessage()); |
There was a problem hiding this comment.
PROPERTY_CONFIG_KEY_USER_MESSAGE
Added custom message property and made it Nullable to avoid validation error (initialized it to a default message)