Implement PublishIdentityUpdate and GetInboxLogs endpoints #377
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
neekolas
reviewed
Apr 22, 2024
| } | ||
|
|
||
| // TODO: Implement serializable isolation level once supported | ||
| if err := s.db.RunInTx(ctx, &sql.TxOptions{ /*Isolation: sql.LevelSerializable*/ }, func(ctx context.Context, tx bun.Tx) error { |
There was a problem hiding this comment.
Why is every tool for working with SQL in Go so terrible?
There was a problem hiding this comment.
I hate Bun. I hate Gorm. Sqlx is fine but very bare-bones.
I've heard good things about sqlc but never tried it
There was a problem hiding this comment.
Awesome, this will be a good list to try when we come back to this. Always hate working with ORM's
neekolas
reviewed
Apr 22, 2024
| for i, req := range reqs { | ||
| inbox_log_entries := make([]*InboxLogEntry, 0) | ||
|
|
||
| err := s.db.NewSelect(). |
There was a problem hiding this comment.
Would be nice to get all of these in a single SQL query, but we can optimize that later.
neekolas
approved these changes
Apr 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stubbed out the validation service for now, as well as the
GetInboxIdsendpoint - these will be implemented in the next PR. Also will add more efficient InboxID encoding, and idempotent publishes, shortly.Notes:
I did not split the logic between 'store' and 'service' layers. It seems 90% of the logic would be in the database anyway, and it felt like splitting it up would make it harder to optimize later. Happy to refactor if anyone disagrees though.
Our ORM Bun does not currently support custom isolation levels for Postgres. Decided to punt this for now - using SERIALIZABLE isolation level just prevents races when two
CreateInboxupdates are published simultaneously, which should be mostly prevented by our hash validation mechanism anyway.