-
Notifications
You must be signed in to change notification settings - Fork 160
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
WIP: Adding initial support for some simple Kafka "mapping #26
Conversation
Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>
final Map<K, V> rawKafkaRecord = new HashMap(); | ||
rawKafkaRecord.put(record.key(), record.value()); | ||
builder.data(rawKafkaRecord); | ||
|
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.
NOTE: I am not using an "event key" here - instead, I put the key/value pair to the DATA
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.
We really need to revolve eventKey use. @matzew - can you jump on the eventKey PR discussion where we can discuss further?
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.
LGTM
case DEFAULT: return new V02HttpTransportMappers(); | ||
} | ||
|
||
// you should not be 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.
🤣
builder.data(rawKafkaRecord); | ||
|
||
// we hard wire to this type: | ||
builder.contentType("application/kafka"); // todo: move to constant |
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.
What does it mean to have a content type of "application/kafka"? Isn't the transport an orthogonal concern to the content type?
builder.contentType("application/kafka"); // todo: move to constant | ||
|
||
} catch (Exception e) { | ||
e.printStackTrace(); |
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.
Better raise it?
Whats the status of this PR? |
waiting for spec to move kafka transport in
or.. we merge as experimenatal ?
On Wed 3. Apr 2019 at 23:29, tarunrathor-pro ***@***.***> wrote:
Whats the status of this PR?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#26 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJnzmcf03_0I_Z1oGUqLJ_HRpHNMHQDks5vdRtGgaJpZM4ZQFwX>
.
--
Sent from Gmail Mobile
|
Great news! btw, is this branch also version 0.3 compliant? |
btw - I have been investigating how to implement the Kafka transport binding properly, it may use part of this PR. |
Any updates on this PR? It seems like |
Solved in the PR #49 Let's close this? |
+1
On Wed 25. Sep 2019 at 20:23, Fabio José ***@***.***> wrote:
Solved in the PR #49 <#49>
Let's close this?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#26?email_source=notifications&email_token=AABGPTWYB7TG7EZFKJIJBN3QLOUALA5CNFSM4GKALQL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7S3YYA#issuecomment-535149664>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABGPTSB5OQGDJ2JQCZFLC3QLOUALANCNFSM4GKALQLQ>
.
--
Sent from Gmail Mobile
|
Available in version 0.3.0 |
Signed-off-by: Matthias Wessendorf mwessend@redhat.com
support for Apache Kafka, as discussed here #5
and here: cloudevents/spec#337
First POC, @bluemonk3y - with some tests ... more to come, and to polished