[
https://issues.apache.org/jira/browse/SAMZA-471?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14234719#comment-14234719
]
Chris Riccomini commented on SAMZA-471:
---------------------------------------
Overall, I like this change. I left some notes on the RB. High level notes:
# ConfigDef looks good (surprise! :)). If anyone has feedback on copy vs.
depend, please speak up. I'm very pro-copy/paste, rather than depending on
Kafka (where the class was stolen from) because I don't want to introduce a
Kafka dependency in samza-core or samza-api.
# It looks like implicit is still there. I'm guessing you just haven't removed
it yet, correct?
# Regarding up-front validation, it'd be nice to instantiate all factories in
either JobRunner or JobCoordinator to trigger validation. Can be a subtask
ticket.
# It'd be cool to have a gradle task that generates the config docs. Can be a
subtask ticket.
# Cleaning up the getOption().getOrElse code in the actual config object might
make for a better user experience. Having a single method (instead of
getOption) that automatically fills out the SamzaException if no config is
available, and maybe prints (doc information as well) would be pretty useful.
That way, when a config is missing, you know which one (in a standardized why)
AND you get the docs on what the config is right in the logs.
In general, code needs some cleanup, but since this is just an early-review
patch, I didn't bother too much with it.
> Make config easy to specify default values and validation
> ---------------------------------------------------------
>
> Key: SAMZA-471
> URL: https://issues.apache.org/jira/browse/SAMZA-471
> Project: Samza
> Issue Type: Improvement
> Affects Versions: 0.9.0
> Reporter: Chinmay Soman
> Assignee: Chinmay Soman
> Fix For: 0.9.0
>
> Attachments: samza-471_0.patch
>
>
> This ticket addresses the first few points of SAMZA-40:
> * Want to auto-generate documentation based off of configuration.
> * Should support global defaults for a config property. Right now, we do
> config.getFoo.getOrElse() everywhere.
> * Should validate config up front, rather than thrown runtime exceptions
> randomly throughout the code.
> * Should remove implicits
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
