Skip to content

Add BackgroundProcessor::start_without_om #3612

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Open
TheBlueMatt opened this issue Feb 21, 2025 · 5 comments · May be fixed by #3688
Open

Add BackgroundProcessor::start_without_om #3612

TheBlueMatt opened this issue Feb 21, 2025 · 5 comments · May be fixed by #3688
Milestone
0.2

Comments

@TheBlueMatt
Copy link
Collaborator

BackgroundProcessor::start takes an Option for the OnionMessenger which is quite annoying as it requires a concrete type for the OnionMessenger even when passing None. We should add a new constructor to make this easier for users.
@TheBlueMatt TheBlueMatt added this to the 0.2 milestone Feb 21, 2025
@Anyitechs
Copy link
Contributor

Hi @TheBlueMatt, I'll like to work on this issue. Can it be assigned to me please?

@TheBlueMatt
Copy link
Collaborator Author

Go for it!

@Anyitechs
Copy link
Contributor

Go for it!

Thank you @TheBlueMatt. On it!

@tnull
Copy link
Contributor

tnull commented Mar 3, 2025

It might be a bit late to mention this, but given that we'll need to introduce similar variants for LiquidiityManager in/post #3509, I wonder if we should use a builder pattern or have the BP take a BackgroundProcessorConfig or similar?

Otherwise the API might get a bit messy if we add constructors for all combinations of parameters?

@Anyitechs
Copy link
Contributor

It might be a bit late to mention this, but given that we'll need to introduce similar variants for LiquidiityManager in/post #3509, I wonder if we should use a builder pattern or have the BP take a BackgroundProcessorConfig or similar?

Otherwise the API might get a bit messy if we add constructors for all combinations of parameters?

Good call @tnull. IMO I believe the builder pattern is the most flexible solution that can easily accommodate future additions. I'll open a PR for this mid next week.

@Anyitechs Anyitechs linked a pull request Mar 28, 2025 that will close this issue
Open
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.2
3 participants
@tnull @TheBlueMatt @Anyitechs