Skip to content

feat(eventarc): new sample for storage event receiver #10222

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

Merged
merged 17 commits into from
Jun 28, 2023

Conversation

muncus
Copy link
Contributor

@muncus muncus commented Jun 12, 2023

Description

Create a new sample demonstrating receipt of Cloud Storage events with eventarc
and cloudevents.

Checklist

@product-auto-label product-auto-label bot added api: eventarc Issues related to the Eventarc API. samples Issues that are directly related to samples. labels Jun 12, 2023
@muncus muncus marked this pull request as ready for review June 13, 2023 16:15
@muncus muncus requested review from a team as code owners June 13, 2023 16:15
@snippet-bot
Copy link

snippet-bot bot commented Jun 13, 2023

Here is the summary of changes.

You are about to add 4 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@muncus muncus added the owlbot:run Add this label to trigger the Owlbot post processor. label Jun 14, 2023
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jun 14, 2023
@muncus muncus added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 14, 2023
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 14, 2023
@muncus
Copy link
Contributor Author

muncus commented Jun 14, 2023

The owlbot post-processor failure is unrelated to this PR (underlying error is a formatting issue with an appengine file not touched in this change).

Copy link
Collaborator

@grayside grayside left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for creating this sample. There are a couple open points here as we align around the plan.

@muncus muncus marked this pull request as draft June 15, 2023 23:04
@muncus muncus marked this pull request as ready for review June 20, 2023 22:06
@muncus muncus requested a review from grayside June 21, 2023 17:45
@gcf-owl-bot gcf-owl-bot bot requested a review from a team as a code owner June 21, 2023 17:48
Copy link
Collaborator

@grayside grayside left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a couple more optional comments.


# Gets the GCS bucket name from the CloudEvent data
# Example: "storage.googleapis.com/projects/_/buckets/my-bucket"
storage_obj = StorageObjectData(event.data)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the data isn't formatted correctly? Is the error message at all useful? Will it crash?

I almost think we should validate the event source and type, but regardless we should make sure not to crash the service if a misconfigured event trigger is created.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This throws a ValueError if there's an invalid field (standard proto-plus behavior).
i've added a try/catch for this, so bad input will result in a 400, and added a test case.

@muncus muncus added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 27, 2023
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 27, 2023
@muncus
Copy link
Contributor Author

muncus commented Jun 27, 2023

Owlbot added formatting changes to DLP files to this PR for reasons I do not understand.
These DLP tests appear to have a resource conflict in them, making these tests flaky.

@muncus muncus added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 27, 2023
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 27, 2023
muncus added a commit that referenced this pull request Jun 27, 2023
These tests compete for the same resource when there are multiple concurrent test runs.

This leads to flaky tests as seen on #10222
engelke pushed a commit that referenced this pull request Jun 27, 2023
* add unique string to dlp infotype tests

These tests compete for the same resource when there are multiple concurrent test runs.

This leads to flaky tests as seen on #10222

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

---------

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
@rsamborski rsamborski added the owlbot:ignore instruct owl-bot to ignore a PR label Jun 28, 2023
@rsamborski
Copy link
Member

rsamborski commented Jun 28, 2023

Owlbot added formatting changes to DLP files to this PR for reasons I do not understand.
These DLP tests appear to have a resource conflict in them, making these tests flaky.

@muncus feel free to revert owlbot changes. PR #10310 should protect against similar situations in the future, but just to be sure I've added an owlbot: ignore label.

@muncus muncus merged commit 4687063 into main Jun 28, 2023
@muncus muncus deleted the eventarc_storage_handler branch June 28, 2023 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: eventarc Issues related to the Eventarc API. owlbot:ignore instruct owl-bot to ignore a PR samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants