Skip to content

feat: Add OnBeforeSend hook#1325

Merged
kodiakhq[bot] merged 5 commits intomainfrom
on-before-send-hook
Oct 20, 2023
Merged

feat: Add OnBeforeSend hook#1325
kodiakhq[bot] merged 5 commits intomainfrom
on-before-send-hook

Conversation

@hermanschaaf
Copy link
Contributor

@hermanschaaf hermanschaaf commented Oct 20, 2023

This adds an optional OnBeforeSend hook to plugin clients that choose to implement the interface. When implemented, the plugin client will receive a call before every message is sent, allowing it to modify the message, perform a side-effect, or return an error.

This keeps the potentially general useful for all plugins, with the first use case being to check whether a row should be counted as premium or not in upcoming paid plugins.

@github-actions github-actions bot added the feat label Oct 20, 2023
@github-actions
Copy link

github-actions bot commented Oct 20, 2023

⏱️ Benchmark results

Comparing with c07c486

  • Glob-8 ns/op: 90.87 ⬇️ 0.87% decrease vs. c07c486

@hermanschaaf hermanschaaf changed the title feat: First draft of plugin on before send hook feat: Add OnBeforeSend hook Oct 20, 2023
@github-actions github-actions bot added feat and removed feat labels Oct 20, 2023
@hermanschaaf hermanschaaf marked this pull request as ready for review October 20, 2023 13:32
Copy link
Contributor

@yevgenypats yevgenypats left a comment

Choose a reason for hiding this comment

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

Looks good. had one question ^

for msg := range msgs {
msg, err = s.Plugin.OnBeforeSend(ctx, msg)
if err != nil {
syncErr = fmt.Errorf("failed before sending message: %w", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't recall now but why we break here and not return like in all other places below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I did it this way was that I didn't want it to be treated as an internal error like in the other return cases, but I think it's all the same really

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I'll change it to return just to be clear

@kodiakhq kodiakhq bot merged commit 023ebbc into main Oct 20, 2023
@kodiakhq kodiakhq bot deleted the on-before-send-hook branch October 20, 2023 15:10
hermanschaaf pushed a commit that referenced this pull request Oct 30, 2023
🤖 I have created a release *beep* *boop*
---


##
[4.17.0](v4.16.1...v4.17.0)
(2023-10-30)


### Features

* Add IsPaid flag to table definition
([#1327](#1327))
([ffd14bf](ffd14bf))
* Add OnBeforeSend hook
([#1325](#1325))
([023ebbc](023ebbc))
* Adding a batch updater to allow usage updates to be batched
([#1326](#1326))
([0301ed7](0301ed7))
* Adding quota monitoring for premium plugins
([#1333](#1333))
([b7a2ca5](b7a2ca5))
* Allow sync to be cancelled when in progress
([#1334](#1334))
([6d7be0b](6d7be0b))


### Bug Fixes

* **deps:** Update github.com/cloudquery/arrow/go/v14 digest to 50d3871
([#1337](#1337))
([f15a89d](f15a89d))
* **deps:** Update github.com/cloudquery/arrow/go/v14 digest to f46436f
([#1329](#1329))
([ee24384](ee24384))
* **deps:** Update module github.com/cloudquery/cloudquery-api-go to
v1.4.2 ([#1335](#1335))
([2ecd2a1](2ecd2a1))
* **deps:** Update module github.com/cloudquery/plugin-pb-go to v1.13.0
([#1332](#1332))
([5553f85](5553f85))
* **deps:** Update module github.com/cloudquery/plugin-pb-go to v1.13.1
([#1336](#1336))
([b782ee7](b782ee7))
* **deps:** Update module google.golang.org/grpc to v1.58.3 [SECURITY]
([#1331](#1331))
([43f60c2](43f60c2))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants