Skip to content

Conversation

@ellemouton
Copy link
Member

Here, we add an abstract ActionsDB interface that we will later add a SQL implementation for.

Part of #917

ellemouton added 4 commits May 9, 2025 16:16
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.
@ellemouton ellemouton added sql-ize no-changelog This PR is does not require a release notes entry labels May 9, 2025
@ellemouton ellemouton self-assigned this May 9, 2025
Copy link
Contributor

@ViktorT-11 ViktorT-11 left a comment

Choose a reason for hiding this comment

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

LGTM 🔥!

Copy link
Contributor

@bitromortac bitromortac left a 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)
Copy link
Contributor

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

Copy link
Member Author

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

@ellemouton ellemouton merged commit 57789b0 into lightninglabs:master May 12, 2025
21 of 22 checks passed
@ellemouton ellemouton deleted the sql36 branch May 12, 2025 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog This PR is does not require a release notes entry sql-ize

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants