[Part 5|*] Thread Contexts through payment methods Part 1#10307
Conversation
ziggie1984
changed the base branch from
master
to
elle-payment-sql-series-new
ziggie1984
force-pushed
the
introduce-sql-schema-payments-part-5
branch
from
283469a to
bc81c3a
Compare
ziggie1984
force-pushed
the
introduce-sql-schema-payments-part-5
branch
from
bc81c3a to
7e6cabd
Compare
ziggie1984
force-pushed
the
elle-payment-sql-series-new
branch
4 times, most recently
from
e991cd0 to
6860702
Compare
ziggie1984
force-pushed
the
introduce-sql-schema-payments-part-5
branch
2 times, most recently
from
a5be3ed to
d76ffcd
Compare
ellemouton
left a comment
ellemouton
left a comment
There was a problem hiding this comment.
nice! quick question
| // The context is already cancelled at this point, so we create | ||
| // a new context so the payment can successfully be marked as | ||
| // failed. | ||
| cleanupCtx := context.WithoutCancel(ctx) |
There was a problem hiding this comment.
hm - i wonder if we should make it a timeout context so we dont let it run forever?
There was a problem hiding this comment.
Yes I want this to securly commit to the db without any timeout, if something is wrong with the db it will be shown here. I mean in case the db connection flakes, this call will nonetheless return an error. Introducing like a context with a random timeout felt not good design but open to change it if yy things its better to have a timeout call.
There was a problem hiding this comment.
At this point i think it's easier to merge this PR and then address this question in a new PR, as most part of the PR is just passing the ctx, would be nice to merge first.
ziggie1984
force-pushed
the
elle-payment-sql-series-new
branch
2 times, most recently
from
47606dd to
01d9986
Compare
ziggie1984
force-pushed
the
introduce-sql-schema-payments-part-5
branch
2 times, most recently
from
3f22326 to
86ebeb7
Compare
ziggie1984
force-pushed
the
introduce-sql-schema-payments-part-5
branch
from
86ebeb7 to
736f645
Compare
| // The context is already cancelled at this point, so we create | ||
| // a new context so the payment can successfully be marked as | ||
| // failed. | ||
| cleanupCtx := context.WithoutCancel(ctx) |
There was a problem hiding this comment.
At this point i think it's easier to merge this PR and then address this question in a new PR, as most part of the PR is just passing the ctx, would be nice to merge first.
yyforyongyu
merged commit fcd4345
into
lightningnetwork:elle-payment-sql-series-new
3 participants
Builds on top of the latest payment SQL changes and threads the context through payment functions.
This is part 1 threads the context up to the Router Level. We can even further thread the context up to the rpc level, however we need to be careful which functions we allow to be canceled by the RPC level. But this is tackled in Part 2.