Fix handling of topics with periods #70
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Can one of the admins verify this patch? |
|
Hey @jingw, It looks like you haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here. Once you've signed reply with Appreciation of efforts, clabot |
|
Our legal folks filled out the CLA. Please let me know if I still need to do something for that. |
| if (!m.matches()) { | ||
| throw new IllegalArgumentException(filename + " does not match COMMITTED_FILENAME_PATTERN"); | ||
| } | ||
| return Long.parseLong(m.group(4)); |
There was a problem hiding this comment.
Should we maybe change this to a named constant in HdfsSinkConnectorConstants since it could change if, e.g. any grouping was adjusted in COMMITTED_FILENAME_PATTERN?
|
@jingw Looks great. I left a couple of comments about cleanup and duplicated code in a test, but should all be trivial to cleanup and get committed. |
|
test this please |
Currently the filters / offset extraction rely on splitting on dot. If there's a topic called `namespace.topic`, it'll extract `namespace` and compare that with the topic name. This breaks recovery, where it looks for the max offset. Also fixed typo in HdfsSinkConnecorConstants and deleted unused CommittedFileWithEndOffsetFilter.
|
I made the suggested cleanups. Let me know if you need anything else. Thanks! |
|
ok to test |
|
LGTM, thanks for the contribution! |
Merged
|
Did this make it into 3.0.1? I have a topic like |
|
It did not. See you at the next release! |
|
I put an image (from automated build) on Docker Hub from the 3.0.1 images that has a custom build with this patch applied in case anyone needs this fix. |
This was referenced Oct 14, 2016
|
Is it an option to mention custom DB name for Hive instead of using the Kafka topic name.. If this is already covered, could you briefly describe the procedure .. |
serssp
pushed a commit
to serssp/kafka-connect-hdfs
that referenced
this pull request
Dec 29, 2018
…failure MINOR: Fix compilation error due to new method in SinkTaskContext
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently the filters / offset extraction rely on splitting on dot. If there's a topic called
namespace.topic, it'll extractnamespaceand compare that with the topic name. This breaks recovery, where it looks for the max offset.Also fixed typo in HdfsSinkConnecorConstants and deleted unused CommittedFileWithEndOffsetFilter.
Closes #45