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

Enhance aiRunnerImage flag #3284

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

RUFFY-369
Copy link

@RUFFY-369 RUFFY-369 commented Nov 28, 2024

What does this pull request do? Explain your changes. (required)
Towards closing 'livepeer/bounties/#69

This PR improves the aiRunnerImage flag to accept both string and JSON string inputs by adding the doc. Functionality is on the ai-worker side.

Usage: aiRunnerImage="{'segment-anything-2': 'livepeer/ai-runner:segment-anything-2-v1.0'}"

Specific updates (required)

  • Usage doc has been added for the flag

How did you test each of these updates (required)

Test was performed by starting go-livepeer as aiWorker with aiRunnerImage="{'segment-anything-2': 'livepeer/ai-runner:segment-anything-2-v1.0'}" and verified launch version.

Does this pull request close any open issues?

Checklist:

cc @rickstaa

Copy link

codecov bot commented Nov 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 33.75271%. Comparing base (1dfe3fd) to head (3089299).

Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##              master       #3284         +/-   ##
===================================================
- Coverage   33.75539%   33.75271%   -0.00268%     
===================================================
  Files            141         141                 
  Lines          37357       37357                 
===================================================
- Hits           12610       12609          -1     
- Misses         24027       24028          +1     
  Partials         720         720                 
Files with missing lines Coverage Δ
cmd/livepeer/livepeer.go 55.65217% <100.00000%> (ø)

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1dfe3fd...3089299. Read the comment docs.

Files with missing lines Coverage Δ
cmd/livepeer/livepeer.go 55.65217% <100.00000%> (ø)

... and 1 file with indirect coverage changes

@@ -160,7 +160,7 @@ func parseLivepeerConfig() starter.LivepeerConfig {
cfg.AIWorker = flag.Bool("aiWorker", *cfg.AIWorker, "Set to true to run an AI worker")
cfg.AIModels = flag.String("aiModels", *cfg.AIModels, "Set models (pipeline:model_id) for AI worker to load upon initialization")
cfg.AIModelsDir = flag.String("aiModelsDir", *cfg.AIModelsDir, "Set directory where AI model weights are stored")
cfg.AIRunnerImage = flag.String("aiRunnerImage", *cfg.AIRunnerImage, "Set the docker image for the AI runner: Example - livepeer/ai-runner:0.0.1")
cfg.AIRunnerImage = flag.String("aiRunnerImage", *cfg.AIRunnerImage, "Set the docker image for the AI runner: Example - livepeer/ai-runner:0.0.1 . You can also provide a JSON string to map specific pipeline names to custom images, allowing for pipeline-specific image overrides: Example - '{\"segment-anything-2\": \"livepeer/ai-runner:segment-anything-2-v1.0\", \"another-pipeline\": \"livepeer/ai-runner:another-pipeline-v2.0\"}'")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
cfg.AIRunnerImage = flag.String("aiRunnerImage", *cfg.AIRunnerImage, "Set the docker image for the AI runner: Example - livepeer/ai-runner:0.0.1 . You can also provide a JSON string to map specific pipeline names to custom images, allowing for pipeline-specific image overrides: Example - '{\"segment-anything-2\": \"livepeer/ai-runner:segment-anything-2-v1.0\", \"another-pipeline\": \"livepeer/ai-runner:another-pipeline-v2.0\"}'")
cfg.AIRunnerImage = flag.String("aiRunnerImage", *cfg.AIRunnerImage, `Set the docker image for the AI runner: Example - livepeer/ai-runner:0.0.1 . You can also provide a JSON string to map specific pipeline names to custom images, allowing for pipeline-specific image overrides: Example - '{"segment-anything-2": "livepeer/ai-runner:segment-anything-2-v1.0", "another-pipeline": "livepeer/ai-runner:another-pipeline-v2.0"}'`)

Copy link
Member

Choose a reason for hiding this comment

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

@RUFFY-369 looks good! However, maybe we can use raw string literals to cleanup the code a bit?

Copy link
Author

Choose a reason for hiding this comment

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

@rickstaa Just pushed the commit for the suggestion. Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

Thanks approved!

Copy link
Member

@rickstaa rickstaa left a comment

Choose a reason for hiding this comment

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

LGTM 🙏🏻.

@ad-astra-video
Copy link
Collaborator

@rickstaa @RUFFY-369 Are we waiting on anything to merge this PR?

@RUFFY-369
Copy link
Author

@rickstaa @RUFFY-369 Are we waiting on anything to merge this PR?

@ad-astra-video In my opinion, I don't think so. We can have @rickstaa 's thought if he has something in mind before this merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants