[
https://issues.apache.org/jira/browse/FLUME-1181?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13269037#comment-13269037
]
[email protected] commented on FLUME-1181:
------------------------------------------------------
bq. On 2012-05-05 10:51:23, Hari Shreedharan wrote:
bq. >
/trunk/flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java,
line 129
bq. > <https://reviews.apache.org/r/5017/diff/1/?file=106967#file106967line129>
bq. >
bq. > Same as TCP source, we might want to handle NumberFormatExceptions
for all getInteger calls. Maybe we should do that in Context itself.
bq.
bq. Brock Noland wrote:
bq. I prefer throwing any NFE from context as it can silently hide
problems where you thought the configuration was set appropriately. I think
it's better to fail to start to let the user know things are not correct as
opposed to continuing with a configuration they did not desire.
bq.
bq. Hari Shreedharan wrote:
bq. Brock - I was not suggesting that we not throw from Context. We could
wrap them up and throw, but on the other hand it is better that the calling
code would know exactly why it failed without using getCause(). Either way is
fine, I suggested this because of the fact that every getInteger call seems to
need an NFE catch clause, but if it isnt this is will still need a catch with
FlumException.
We might be communicating the same thing in different words. I'd prefer that
it's completely thrown from the configure method and the configure caller logs
and disables the sink/source/channel. This way it won't work at all and someone
will immediately know there is a problem as opposed to having to look at the
logs to see if the configuration changes were applied.
- Brock
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5017/#review7595
-----------------------------------------------------------
On 2012-05-05 06:22:46, Arvind Prabhakar wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/5017/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-05-05 06:22:46)
bq.
bq.
bq. Review request for Flume, Brock Noland and Hari Shreedharan.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. This change:
bq. * introduces a precondition check in Context.getSubProperties(String
prefix) method enforce that the prefix ends with a period,
bq. * fixes the TestPropertiesFileConfigurationProvider to load the property
file which was previously not working, and fixes cases that are broken
bq. * refactors some of the source (not all) to externalize the configuration
keys into separate constants class
bq.
bq.
bq. This addresses bug FLUME-1181.
bq. https://issues.apache.org/jira/browse/FLUME-1181
bq.
bq.
bq. Diffs
bq. -----
bq.
bq.
/trunk/flume-ng-configuration/src/main/java/org/apache/flume/Context.java
1334310
bq.
/trunk/flume-ng-configuration/src/main/java/org/apache/flume/conf/BasicConfigurationConstants.java
PRE-CREATION
bq.
/trunk/flume-ng-configuration/src/main/java/org/apache/flume/conf/ComponentConfiguration.java
1334310
bq.
/trunk/flume-ng-configuration/src/main/java/org/apache/flume/conf/FlumeConfiguration.java
1334310
bq.
/trunk/flume-ng-configuration/src/main/java/org/apache/flume/conf/sink/SinkGroupConfiguration.java
1334310
bq.
/trunk/flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceConfiguration.java
1334310
bq.
/trunk/flume-ng-core/src/main/java/org/apache/flume/channel/ChannelSelectorFactory.java
1334310
bq.
/trunk/flume-ng-core/src/main/java/org/apache/flume/channel/MultiplexingChannelSelector.java
1334310
bq.
/trunk/flume-ng-core/src/main/java/org/apache/flume/source/SyslogSourceConfigurationConstants.java
PRE-CREATION
bq.
/trunk/flume-ng-core/src/main/java/org/apache/flume/source/SyslogTcpSource.java
1334310
bq.
/trunk/flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java
1334310
bq.
/trunk/flume-ng-core/src/main/java/org/apache/flume/source/SyslogUtils.java
1334310
bq.
/trunk/flume-ng-node/src/main/java/org/apache/flume/conf/properties/PropertiesFileConfigurationProvider.java
1334310
bq.
/trunk/flume-ng-node/src/test/java/org/apache/flume/conf/properties/TestPropertiesFileConfigurationProvider.java
1334310
bq. /trunk/flume-ng-node/src/test/resources/flume-conf.properties 1334310
bq.
bq. Diff: https://reviews.apache.org/r/5017/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Ran all tests. The updated TestPropertiesFileConfigurationProvider catches
two issues now - the one with channel selector configuration not being set
correctly, and the other a similar issue with syslog source format
configuration. Both of these issues have been fixed with the changes.
bq.
bq. Also done some manual validation of the system with a few simple scenarios.
bq.
bq.
bq. Thanks,
bq.
bq. Arvind
bq.
bq.
> Context must enforce dot-separated prefix for sub-properties.
> -------------------------------------------------------------
>
> Key: FLUME-1181
> URL: https://issues.apache.org/jira/browse/FLUME-1181
> Project: Flume
> Issue Type: Bug
> Components: Configuration
> Reporter: Arvind Prabhakar
> Assignee: Arvind Prabhakar
> Fix For: v1.2.0
>
> Attachments: FLUME-1181-1.patch
>
>
> Currently the method Context.getSubProperties(String prefix) assumes a
> well-formed prefix. This can lead to undetected issues such as what is being
> seen in the current state of channel selector configuration, where the keys
> within the channel selector are being computed as prefixed with a period.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
