APEXMALHAR-2379 Validation of attributes for AbstractFileOutputOperat… #530
Conversation
|
Why is this not done using the javax.validation constraint annotations ? |
| @@ -366,6 +367,9 @@ public int getScanIntervalMillis() | |||
| */ | |||
| public void setScanIntervalMillis(int scanIntervalMillis) | |||
| { | |||
| if (scanIntervalMillis < 0) { | |||
| throw new IllegalArgumentException("scanIntervalMillis should be greater than or equal to 0."); | |||
| } | |||
There was a problem hiding this comment.
Can we add the validation annotation to scanIntervalMillis, instead fixing it in setter method ?
| @@ -384,6 +388,9 @@ public int getEmitBatchSize() | |||
| */ | |||
| public void setEmitBatchSize(int emitBatchSize) | |||
| { | |||
| if (emitBatchSize <= 0) { | |||
| throw new IllegalArgumentException("emitBatchSize should be greater than 0."); | |||
| } | |||
There was a problem hiding this comment.
Can we add the validation annotation to emitBatchSize, instead fixing it in setter method ?
| @@ -994,6 +1001,9 @@ public int getMaxRetryCount() | |||
| */ | |||
| public void setMaxRetryCount(int maxRetryCount) | |||
| { | |||
| if (maxRetryCount < 0) { | |||
| throw new IllegalArgumentException("maxRetryCount should be greater than or equal to 0."); | |||
| } | |||
There was a problem hiding this comment.
@tweise @chaithu14
I took the change in https://github.com/apache/apex-malhar/blob/master/library/src/main/java/org/apache/apex/malhar/lib/fs/FSRecordReader.java#L158
I had tried the @min constraint annotation, which throws a constraint violation exception if the values are invalid in the properties.xml (bean validation). However, I do not see it throwing exceptions during the setting of the property of the operator when the application is running. Please let me know if I have misunderstood something here.
There was a problem hiding this comment.
Are talking about dynamic property changes ? If yes, I think we need to fix it up in core.
There was a problem hiding this comment.
@chaithu14 agreed. Will raise a ticket for the same on APEXCORE. Can the existing changes be merged? I've added the @ Min constraint annotation so that it can be picked up.
There was a problem hiding this comment.
@chaithu14 Created APECORE-600
| @@ -117,7 +117,7 @@ public void testSinglePartiton() throws Exception | |||
| oper.output.setSink(sink); | |||
|
|
|||
| oper.setDirectory(testMeta.dir); | |||
| oper.getScanner().setFilePatternRegexp(".*file[\\d]"); | |||
| oper.getScanner().setFilePatternRegexp("((?!target).)*file[\\d]"); | |||
francisf
force-pushed
the
APEXMALHAR-2379_Negative_vals,regex_file
branch
2 times, most recently
from
6a56456
to
6ada96b
Compare
…or, regex on name instead of path, added @ Min constraint annotation
francisf
force-pushed
the
APEXMALHAR-2379_Negative_vals,regex_file
branch
from
6ada96b
to
16b15c2
Compare
|
Looks good to me. |
…or, regex on name instead of path
@bhupeshchawda please review