-
Notifications
You must be signed in to change notification settings - Fork 1
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
CI. Dummy runtime. Dummy offer. Miners removed. #12
Conversation
f3be99b
to
3c84c3c
Compare
When tagged CI produces release with |
I need to add |
ec0886e
to
49307a5
Compare
let cli = match Cli::try_parse() { | ||
Ok(cli) => cli, | ||
Err(err) => { | ||
log::error!("Failed to parse CLI: {}", err); | ||
err.exit(); | ||
} | ||
}; | ||
|
||
match cli.runtime.to_lowercase().as_str() { | ||
"dummy" => run::<process::dummy::Dummy>(cli).await, | ||
_ => { | ||
let err = anyhow::format_err!("Unsupported framework {}", cli.runtime); | ||
log::error!("{}", err); | ||
anyhow::bail!(err) | ||
} |
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.
Instead of logging error and returning it, you could create 2-levels main function: external will print error message and internal will just use ?
operator.
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.
This bail
is really pointless because ya-provider
does not save supervisor/exe-unit
process output. I added logs only because I could not determine why process is failing (and I could not start it outside of ya-provider context because it requires gsb endpoint to start). This block of code will need to be rewritten the moment we will add new runtime/ai_framework/package.
let capabilities = vec![serde_json::Value::String(runtime_name)]; | ||
template.properties.insert( | ||
"golem.runtime.capabilities".to_string(), | ||
serde_json::Value::Array(capabilities), | ||
); |
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.
Runtime
name is added as only capability, but we should list all possible runtimes here. And probably we will need more descriptive structure than just list.
But this isn't important now. Let's leave it as is.
Resolves: #8