Skip to content
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

Refactor livepeer.go to enable running Livepeer node from the code #2351

Merged
merged 2 commits into from
Apr 8, 2022

Conversation

leszko
Copy link
Contributor

@leszko leszko commented Apr 5, 2022

What does this pull request do? Explain your changes. (required)

Refactor livepeer.go to allow running Livepeer node from the code. After this change, you can start Livepeer node from the code (without passing any flags) in the following manner.

// Start Livepeer node
go func() {
        // Sample configuration for combined O/T topology
	datadir := "~/temp/livepeer/TEST_OT"
	orchestrator := true
	transcoder := true
	serviceAddr := "127.0.0.1:8935"
	cfg := DefaultLivepeerConfig()
	cfg.datadir, cfg.orchestrator, cfg.transcoder, cfg.serviceAddr = &datadir, &orchestrator, &transcoder, &serviceAddr

        // Starting the Livepeer node
	StartLivepeer(ctx, cfg)
}()

The context for this change is the work on implementing E2E Testing which would cover interactions with the chain. Additionally, I believe it's a way better code structure when we split the configuration and the flags parsing.

Some ideas for future improvements (not included in this PR):

  • Allow running Livepeer as library for end-users (improve LivepeerConfig structure)
  • Refactor main() to extract it into functions like parseOrchAddrs()
  • Make sure it's possible to run multiple Livepeer instances in one process (in general it works, but may need some tweaks)

Specific updates (required)

  • Create the LivepeerConfig type and the DefaultLivepeerConfig() function
  • Change parsing flags to fill LivepeerConfig
  • Set nils in LivepeerConfig instead of using the isFlagSet map
  • Extract parseOrchAddrs() function

How did you test each of these updates (required)

Started Livepeer on-chain and off-chain and played with transcoding and CLI interactions.

Does this pull request close any open issues?

No, but it's related to #2317

Checklist:

Copy link
Contributor

@thomshutt thomshutt left a comment

Choose a reason for hiding this comment

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

Looks good, I like the ideas for future steps too


// updateNilsForUnsetFlags changes some cfg fields to nil if they were not explicitly set with flags.
// For some flags, the behavior is different whether the value is default or not set by the user at all.
func updateNilsForUnsetFlags(cfg LivepeerConfig) LivepeerConfig {
Copy link
Contributor

@thomshutt thomshutt Apr 6, 2022

Choose a reason for hiding this comment

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

👍 I've always found it annoying that the Go flags package doesn't give a way to distinguish between zero value / not set, but good to at least do it all in one place

@leszko leszko force-pushed the 2317-refactor-livepeer-starter branch from e64df90 to d45905f Compare April 8, 2022 11:59
@leszko leszko merged commit 0cceff1 into livepeer:master Apr 8, 2022
@leszko leszko deleted the 2317-refactor-livepeer-starter branch April 8, 2022 12:48
ad-astra-video pushed a commit to ad-astra-video/go-livepeer that referenced this pull request May 25, 2022
@leszko leszko mentioned this pull request Jul 11, 2022
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants