-
Notifications
You must be signed in to change notification settings - Fork 455
Gcp pubsub source #3720
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
Gcp pubsub source #3720
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.
I tried to applied all your comment @guilload. Let me know if I misunderstood something.
In next PR, I will try to add the ACK of the message with some test.
Then I will try to add parallelism if needed, and some integration test.
64d9e70
to
caf10de
Compare
* Initial pubsub implementation/test * Allow pubsub to have multiple pipelines
ca83a7b
to
aa33d1e
Compare
7f77157
to
cf1965b
Compare
Merged! Awesome job @magnalite and @AyWa. It's really great to land large features from OSS contributors. @AyWa, as a next step, I would welcome a PR that adds some tests before adding more new functionalities to the source (parallelism, ack on truncate). |
@AyWa @magnalite Ping me on Discord if you want some swag :) |
Description
Continuation of the great work of @magnalite and following advice of @guilload
This PR, fix the compilation/formatting issues of the previous PR and put the code behind a flag
gcp-pubsub
Also renamed everywhere
pubsub
bygcp-pubsub
orGcpPubSub
to reduce confusion with other pubsub system.How was this PR tested?
I didnt test it. It will be added in step 3
Next Item
step 2: focus on supporting at-least-once delivery semantics for now and improve the current implementation as you suggested
step 3: add unit and integration tests for this source
step 4: test at scale (I can help with that and do so on our GCP account)
step 5: opportunistically support exactly-once delivery semantics