-
Notifications
You must be signed in to change notification settings - Fork 1.6k
FLUME-3023 {variable} substitution doesn't work for property 'fileSuffix' #110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new test seems to be passing without applying the changes to HDFSEventSink.java .
Can you please double-check the code?
String translatedSuffixString = BucketPath.escapeString(SUFFIX, e.getHeaders()); | ||
|
||
BucketWriter bucketWriter = new BucketWriter( | ||
ROLL_INTERVAL, 0, 0, 0, ctx, "/tmp", "file", "", translatedSuffixString, null, null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't SUFFIX
to be supplied here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ignore this test I will write a new one ... this seems to be incorrect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please provide a minimal changeset? (Ie. remove all formatting and whitespace-only changes, in order to help with the review and have a clean patch.)
|
||
// check that the roll happened correctly for the given data | ||
Assert.assertTrue("File suffix translation failed: " + fList[0].toUri(), | ||
fList[0].getName().indexOf(translatedSuffixString) != -1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be rewritten as fList[0].getName().contains(translatedSuffixString)
. Also, I think it should be fList[0].getName().endsWith(translatedSuffixString)
sink.stop(); | ||
|
||
String translatedSuffixString = | ||
BucketPath.escapeString(context.getString("hdfs.fileSuffix"), event.getHeaders()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we hard-code the value of translatedSuffixString
? (I think it is always "_20170201.avro"
)
}; | ||
Clock prevClock = BucketPath.getClock(); | ||
|
||
BucketPath.setClock(testClock); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is setting the clock necessary?
Can one of the admins verify this patch? |
rtmp-play log hide node ip
Escaped the suffix before passing it to bucket writer.