Skip to content
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

Merged
merged 6 commits into from
Aug 11, 2023
Merged

Gcp pubsub source #3720

merged 6 commits into from
Aug 11, 2023

Conversation

AyWa
Copy link
Collaborator

@AyWa AyWa commented Aug 7, 2023

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 by gcp-pubsub or GcpPubSub 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

@AyWa AyWa self-assigned this Aug 7, 2023
@AyWa AyWa requested a review from guilload August 7, 2023 15:07
Copy link
Collaborator Author

@AyWa AyWa left a 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.

quickwit/quickwit-indexing/src/source/gcp_pubsub_source.rs Outdated Show resolved Hide resolved
@AyWa AyWa force-pushed the gcp-pubsub-source branch 2 times, most recently from 64d9e70 to caf10de Compare August 10, 2023 13:46
@AyWa AyWa requested a review from guilload August 10, 2023 14:36
@AyWa AyWa requested a review from guilload August 11, 2023 14:09
@guilload guilload enabled auto-merge (squash) August 11, 2023 14:36
@guilload guilload merged commit c692807 into main Aug 11, 2023
3 checks passed
@guilload guilload deleted the gcp-pubsub-source branch August 11, 2023 15:51
@guilload
Copy link
Member

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).

@fmassot
Copy link
Contributor

fmassot commented Aug 11, 2023

@AyWa @magnalite Ping me on Discord if you want some swag :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants