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

Adds pgt_outbox_setup and pgt_outbox_events for Transactional Outbox Pattern #19

Closed
wants to merge 7 commits into from

Conversation

bougyman
Copy link

This adds two new methods, which implement the transactional
outbox pattern utilizing postgresql triggers.

This only handles creating the outbox table, and writing events
to the outbox for table changes, no other part of the system.

Context:

* feat: Adds transactional outbox setup and trigger

* docs: Adds README section about outbox trigger

* test: Adds spec for outbox trigger

* test: Completes basic spec for outbox

* test: Gets specs back to 100% line/branch coverage
Copy link
Owner

@jeremyevans jeremyevans left a comment

Choose a reason for hiding this comment

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

Thank you for working on this. There seems to be a significant amount of inherent complexity (17 options!). I fear it may be too specialized to your needs, even though I can see you've spent significant time trying to make it more generalized (hence the large number of options). I'm also not sure usage of this feature would be high enough to justify including it. However, I could see it saving someone a lot of time if this meets their needs. So I'm on the fence about whether to merge it.

In terms of implementation, testing looks good, modulo unnecessary whitespace changes. Forcing timestamptz is probably not desirable. It's better to use Time, and have users that want timestamptz instead of timestamp use the pg_timestamptz extension.

@bougyman
Copy link
Author

bougyman commented Feb 13, 2025

In terms of implementation, testing looks good, modulo unnecessary whitespace changes. Forcing timestamptz is probably not desirable. It's better to use Time, and have users that want timestamptz instead of timestamp use the pg_timestamptz extension.

Made the switch to Time here, thank you for that suggestion, I was unaware of pg_timestamptz.

In terms of options, I wanted to support the various 'outbox/inbox' frameworks I've seen (like tobox, but not limited to it), and this set of 17 options (10 of these are simply column names) with the two conditionals supports almost all that I've seen during my research the last few weeks. And of course it acts very sane and normal in the Basic setup passing only the name of the table to fire outbox events for. I hope to see it included as we already leverage this gem for other triggery behaviors throughout our architecture.

modulo unnecessary whitespace changes

Put the whitespace back like it was :)

@bougyman bougyman closed this Feb 13, 2025
@bougyman bougyman reopened this Feb 13, 2025
@bougyman
Copy link
Author

There wasn't much shared with the rest of this library, so I'm going to publish this as its own gem instead. Thanks for the consideration. https://github.com/rubyists/sequel-pgt_outbox

@bougyman bougyman closed this Feb 15, 2025
@jeremyevans
Copy link
Owner

Sounds good. Might be useful to link to your gem from Sequel's plugins/extensions page (https://sequel.jeremyevans.net/plugins.html), so send a PR to the Sequel repository if you would like to do that.

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.

3 participants