Support timestamp based consumption via Last-Event-ID header
ClosedPublic

Authored by Ottomata on May 30 2018, 7:55 PM.

Details

Reviewers
Pchelolo
Ottomata
Commits
rWKSE25935735308c: Support timestamp based consumption via Last-Event-ID header
Patch without arc
git checkout -b D1063 && curl -L https://phabricator.wikimedia.org/D1063?download=true | git apply
Summary

This updates to node-rdkafka 2.3.3 and bumps KafkaSSE version to 0.2.0.

Bug: T196009

Diff Detail

Repository
rWKSE KafkaSSE
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
Ottomata created this revision.May 30 2018, 7:55 PM
Ottomata requested review of this revision.May 30 2018, 7:58 PM

Hold on review. I have another patch coming as soon as I get tests to pass.

Ottomata updated this revision to Diff 2803.Jun 1 2018, 3:41 PM
  • Simplify offset/timestamp assignment resolution
Pchelolo added inline comments.Jun 4 2018, 2:23 PM
Dockerfile
15

Why do we have a committed Dockerfile? Other services gitignore it and generate it using the service-runner deploy script..

README.md
20

A assignments doesn't sound right.

200

There's a missing space.

test/KafkaSSE.js
237

This is probably not needed.

Ottomata marked 3 inline comments as done.Jun 4 2018, 3:59 PM
Ottomata added inline comments.
Dockerfile
15

KafkaSSE does not use service-runner. This is a standalone library. The Dockerfile is used for tests.

Ottomata updated this revision to Diff 2804.Jun 4 2018, 4:10 PM
  • Fix review coments
Ottomata accepted this revision.Jun 4 2018, 4:30 PM
This revision is now accepted and ready to land.Jun 4 2018, 4:30 PM
This revision was automatically updated to reflect the committed changes.