-
Notifications
You must be signed in to change notification settings - Fork 109
[sql-36] actions: prepare ActionsDB interface #1069
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
Conversation
So that we can easily add a different implementation and swop them out later.
The current ActionLocator is very specific to how actions are stored in the bbolt db. In our SQL implementation, we will simply have an auto-incrementing int64 that we will use as our locator for any action. In preparation for this, we make ActionLocator an abstract interface and implement our bbolt version of it.
Use a helper to compare Actions and prepare the timestamp comparisons so that they are ready for our SQL implementation of the Actions DB.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🔥!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎉
| require.Len(t, actions, 0) | ||
|
|
||
| id, err := db.AddAction(ctx, action1) | ||
| _, err = db.AddAction(ctx, action1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps we could still check uniqueness of the locator, by replacing isActionLocator with a representation() string that we could compare? that way we would have at least some utility in the interface-definding method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it's ok, im gonna leave it as is cause:
- this is a pretty standard pattern in go
- the correctness of the locator is already tested below when we use locator 2 to update the state of an action
Here, we add an abstract
ActionsDBinterface that we will later add a SQL implementation for.Part of #917