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

feat(pubsub): support message filtering in pstest #9015

Merged
merged 21 commits into from Jan 25, 2024

Conversation

zezhehh
Copy link
Contributor

@zezhehh zezhehh commented Nov 16, 2023

Resolves #8582

The implementation:

  • Parse the filter string using go.einride.tech/aip.
  • Match each message attributes with the filter and remove the message if the match fails.

@zezhehh zezhehh requested review from shollyman and a team as code owners November 16, 2023 08:54
@product-auto-label product-auto-label bot added the size: l Pull request size is large. label Nov 16, 2023
@zezhehh zezhehh changed the title feat(pstest): support message filtering feat(pubsub): support message filtering in pstest Nov 16, 2023
@product-auto-label product-auto-label bot added the api: pubsub Issues related to the Pub/Sub API. label Nov 16, 2023
@andreas-lindfalk
Copy link

@shollyman review por favor...

@product-auto-label product-auto-label bot added the stale: old Pull request is old and needs attention. label Dec 16, 2023
@vlasn
Copy link

vlasn commented Dec 21, 2023

@shollyman Friendly nudge, would love to leverage this.

@product-auto-label product-auto-label bot added stale: extraold Pull request is critically old and needs prioritization. and removed stale: old Pull request is old and needs attention. labels Jan 15, 2024
@hongalex
Copy link
Member

Thank you so much for the PR. Apologies as I somehow didn't see this for the longest time. For the most part the PR looks good but I need to confirm with the team on how to tackle this since we generally try to minimize the number of third party libraries.

@zezhehh
Copy link
Contributor Author

zezhehh commented Jan 22, 2024

@hongalex Hi! Thanks for your reviews. Just added test for escapes characters and rebased to main.

@hongalex
Copy link
Member

Checks are failing because of a minor formatting issue that running go vet should solve.

@zezhehh
Copy link
Contributor Author

zezhehh commented Jan 23, 2024

Checks are failing because of a minor formatting issue that running go vet should solve.

Thanks! Seems it requires another approval to trigger the workflow:)

@hongalex
Copy link
Member

One more vet issue, the exported ValidateFilter method needs a comment there.

@zezhehh
Copy link
Contributor Author

zezhehh commented Jan 23, 2024

One more vet issue, the exported ValidateFilter method needs a comment there.

Added the comment. Couldn't succeed in running vet.sh locally. Hopefully, it works this time.

@hongalex hongalex enabled auto-merge (squash) January 23, 2024 23:39
@hongalex hongalex added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jan 24, 2024
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jan 24, 2024
auto-merge was automatically disabled January 24, 2024 07:54

Head branch was pushed to by a user without write access

@zezhehh
Copy link
Contributor Author

zezhehh commented Jan 24, 2024

Checked the error message stating that TestUpdateFilter failed due to a bad filter string. Already updated! 🙌

(However, the test TestSubscriptionMessageOrdering still fails on my computer in both the main and this branch. It seems unrelated to this PR.)

@hongalex hongalex added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jan 24, 2024
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jan 24, 2024
@hongalex hongalex added kokoro:run Add this label to force Kokoro to re-run the tests. and removed stale: extraold Pull request is critically old and needs prioritization. labels Jan 25, 2024
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jan 25, 2024
@hongalex hongalex merged commit 49231bf into googleapis:main Jan 25, 2024
8 of 9 checks passed
@aalobai1
Copy link

aalobai1 commented Feb 3, 2024

I'm so happy this is merged, thank you gcloud team ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the Pub/Sub API. size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pubsub/pstest: Support for subscription filters
6 participants