-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
multi: extend InvoiceDB
methods with a context argument
#8066
Conversation
5952472
to
81e3565
Compare
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.
Simple prep PR, linter unhappy but otherwise lgtm.
htlcswitch/mock.go
Outdated
func (i *mockInvoiceRegistry) AddInvoice(invoice invoices.Invoice, | ||
paymentHash lntypes.Hash) error { | ||
|
||
_, err := i.registry.AddInvoice(&invoice, paymentHash) | ||
_, err := i.registry.AddInvoice( | ||
context.Background(), &invoice, paymentHash, | ||
) |
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.
Is ctx
not passed through here because AddInvoice
isn't part of the InvoiceDatabase
interface? Stands out reading this that this is the only place where we have context.Background()
.
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.
No I think I just forgot to fixup this call. Done.
e4367e4
to
de1dadd
Compare
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.
✨
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🙏 Thanks for breaking down the PR!
This commit adds a context to InvoiceDB's methods. Along this refactor we also extend InvoiceRegistry methods with contexts where it makes sense. This change is essential to be able to provide kvdb and sqldb implementations for InvoiceDB.
de1dadd
to
aa85972
Compare
In preparation of the SQL based
InvoiceDB
implementation (draft in #8052) we add context to allInvoiceDB
methods. This in turn results in some of theInvocieRegistry
methods also requiring external context passed in.Part of #6288