[
https://issues.apache.org/jira/browse/STORM-1084?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14957557#comment-14957557
]
ASF GitHub Bot commented on STORM-1084:
---------------------------------------
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/785#issuecomment-148164422
This is nothing you did, but now that the validation is much more readable
I am seeing a few things that should probably be fixed, but I am happy to do
them myself on a follow on JIRA
`TOPOLOGY_ISOLATED_MACHINES` needs `@isPositiveNumber`
All of the 'ZMQ_` configs should be deprecated.
`TRANSACTIONAL_ZOOKEEPER_PORT`needs `@isPositiveNumber`
It would be great if we could restrict `TOPOLOGY_LOGGING_SENSITIVITY` to
one of the allowed values "S0", "S1", "S2", "S3"
`TOPOLOGY_SHELLBOLT_MAX_PENDING` needs `@isPositiveNumber`
`TOPOLOGY_TRIDENT_BATCH_EMIT_INTERVAL_MILLIS` needs `@isPositiveNumber`
`TOPOLOGY_MAX_ERROR_REPORT_PER_INTERVAL` and
`TOPOLOGY_ERROR_THROTTLE_INTERVAL_SECS` both seem to need `@isPositiveNumber`
`TOPOLOGY_TRANSFER_BUFFER_SIZE` needs to be `@isPowerOf2`
`TOPOLOGY_ENVIRONMENT` should be `@isMapEntryType(keyType = String.class,
valueType = String.class)`.
`TOPOLOGY_SLEEP_SPOUT_WAIT_STRATEGY_TIME_MS` needs
`@isPositiveNumber(includeZero = true)`
`TOPOLOGY_MAX_SPOUT_PENDING` needs `@isPositiveNumber`
`TOPOLOGY_MAX_TASK_PARALLELISM` needs `@isPositiveNumber`
`WORKER_METRICS` and `TOPOLOGY_WORKER_METRICS` should be
`@isMapEntryType(keyType = String.class, valueType = String.class)`.
`TOPOLOGY_METRICS_CONSUMER_REGISTER` should have a custom validator (you
might not have time to do it, so we might need a follow on JIRA for this).
Something like
`@isListEntryCustom(entryValidatorClasses={MetricRegistryValidator.class})`
MetricRegistryValidator.class needs to check that it is a map, with a
"class" key that points to a string, a "parallelism.hint" key that points to a
positive non-null integer.
`TOPOLOGY_EVENTLOGGER_EXECUTORS` needs `@isPositiveNumber`
`TOPOLOGY_ACKER_EXECUTORS` needs `@isPositiveNumber`
`TOPOLOGY_TASKS` and `TOPOLOGY_WORKERS` need `@isPositiveNumber`
`TASK_CREDENTIALS_POLL_SECS` needs `@isPositiveNumber`
`TASK_REFRESH_POLL_SECS` `TASK_HEARTBEAT_FREQUENCY_SECS`
`WORKER_HEARTBEAT_FREQUENCY_SECS` and `WORKER_RECEIVER_THREAD_COUNT` need
`@isPositiveNumber`
`SUPERVISOR_MONITOR_FREQUENCY_SECS` and
`SUPERVISOR_HEARTBEAT_FREQUENCY_SECS` need `@isPositiveNumber`
`SUPERVISOR_WORKER_SHUTDOWN_SLEEP_SECS` needs `@isPositiveNumber`
`DRPC_HTTP_FILTER_PARAMS` should be `@isMapEntryType(keyType =
String.class, valueType = String.class)`.
`DRPC_INVOCATIONS_THREADS` and `DRPC_INVOCATIONS_PORT` need
`@isPositiveNumber`
`DRPC_QUEUE_SIZE` `DRPC_MAX_BUFFER_SIZE` and `DRPC_WORKER_THREADS` need
`@isPositiveNumber`
`DRPC_AUTHORIZER_ACL` needs to be a Map<String, Map<String, String or
List<String>>>. This too probably needs a custom validator in a follow on JIRA.
`DRPC_PORT` needs `@isPositiveNumber`
`DRPC_HTTPS_PORT` and `DRPC_HTTP_PORT` need `@isPositiveNumber`
`UI_HTTPS_PORT` and `UI_HEADER_BUFFER_BYTES` need `@isPositiveNumber`
`UI_FILTER_PARAMS` should be `@isMapEntryType(keyType = String.class,
valueType = String.class)`.
`LOGVIEWER_HTTPS_PORT` needs `@isPositiveNumber`
`LOGVIEWER_PORT` and `UI_PORT` need `@isPositiveNumber`
`NIMBUS_CREDENTIAL_RENEW_FREQ_SECS` needs `@isPositiveNumber`
`NIMBUS_IMPERSONATION_ACL` needs to be updated, because I don't think Map
of string to map. It is more complex then that.
`NIMBUS_TASK_LAUNCH_SECS` `NIMBUS_SUPERVISOR_TIMEOUT_SECS`
`NIMBUS_INBOX_JAR_EXPIRATION_SECS` `NIMBUS_CLEANUP_INBOX_FREQ_SECS`
`NIMBUS_MONITOR_FREQ_SECS` and `NIMBUS_TASK_TIMEOUT_SECS` need
`@isPositiveNumber`
`NIMBUS_THRIFT_MAX_BUFFER_SIZE` needs `@isPositiveNumber`
`NIMBUS_THRIFT_THREADS` and `NIMBUS_THRIFT_PORT` need `@isPositiveNumber`
I think there are more too so we probably need a follow on JIRA for this.
> Improve Storm config validation process to use java annotations instead of
> *_SCHEMA format
> ------------------------------------------------------------------------------------------
>
> Key: STORM-1084
> URL: https://issues.apache.org/jira/browse/STORM-1084
> Project: Apache Storm
> Issue Type: Improvement
> Components: storm-core
> Reporter: Boyang Jerry Peng
> Assignee: Boyang Jerry Peng
>
> So currently we specify validators:
> public static final String STORM_MESSAGING_NETTY_MIN_SLEEP_MS =
> "storm.messaging.netty.min_wait_ms";
> public static final Object STORM_MESSAGING_NETTY_MIN_SLEEP_MS_SCHEMA =
> ConfigValidation.IntegerValidator;
> A better way to do this is using annotations. Something like:
> @IntegerValidator
> public static final String STORM_MESSAGING_NETTY_MIN_SLEEP_MS =
> "storm.messaging.netty.min_wait_ms";
> Do this has many advantages. For one you can stack multiple annotations:
> @IntegerValidator
> @NotNull
> public static final String STORM_MESSAGING_NETTY_MIN_SLEEP_MS =
> "storm.messaging.netty.min_wait_ms";
> And we don't have to write another validator for strings that cannot be null
> And we can pass parameters into the annotations:
> @PositiveIntegerValidator(notNull=true)
> public static final String DRPC_REQUEST_TIMEOUT_SECS =
> "drpc.request.timeout.secs";
> instead of having to write another validator:
> ConfigValidation.NotNullPosIntegerValidator for checking for not null
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
