Feature/custom counters #1513
Feature/custom counters #1513
Conversation
There was a problem hiding this comment.
I like most of changes you made, but I would like to ask you to focus more on the topic of the PR (Custom counters in this case). Doing so, helps a lot with planning (we might have a high priority for CC feature, but not so high for reformatting of 80+ files) & reviewing.
Please also remember about updating documentation (readme file and/or handbook) when changing CLI or golemsp
I would also like to ask you to revert changes that only reorder the use/import statements.
In this review I skipped the following parts:
core/marketcore/vpncore/servutils/transferutils/networkinggoth_testsexe-unit
| let state = serde_json::to_value(globals_state)?; | ||
| let value = state | ||
| .get(&name) | ||
| .ok_or_else(|| anyhow::anyhow!("Invalid name global state property: {}", name))?; |
There was a problem hiding this comment.
| .ok_or_else(|| anyhow::anyhow!("Invalid name global state property: {}", name))?; | |
| .ok_or_else(|| anyhow::anyhow!("Config property not found: {}", name))?; |
| fn list(config: ProviderConfig) -> anyhow::Result<()> { | ||
| let registry = config.registry()?; | ||
| if let Err(errors) = registry.validate() { | ||
| log::error!("Encountered errors while checking ExeUnits:\n{}", errors); |
There was a problem hiding this comment.
| log::error!("Encountered errors while checking ExeUnits:\n{}", errors); | |
| log::error!("Error encountered during ExeUnits registry validation. errors: {}", errors); |
| use structopt::StructOpt; | ||
|
|
||
| #[derive(StructOpt, Clone, Debug)] | ||
| pub enum ConfigConfig { |
There was a problem hiding this comment.
| pub enum ConfigConfig { | |
| pub enum GeneralConfig { |
| @@ -557,6 +560,7 @@ impl Handler<GetOfferTemplates> for TaskRunner { | |||
| log::info!("Reading offer template for {}", key); | |||
| let string = fut.await?; | |||
| let value = serde_json::from_str(string.as_str())?; | |||
| log::info!("offer-template: {} = {:?}", key, value); | |||
There was a problem hiding this comment.
| log::info!("offer-template: {} = {:?}", key, value); | |
| log::debug!("offer template: {} = {:?}", key, value); |
| @@ -383,9 +311,17 @@ where | |||
| } | |||
|
|
|||
| fn default_plugins() -> PathBuf { | |||
| if let Some(mut exe) = env::current_exe().ok() { | |||
There was a problem hiding this comment.
If we want to add this functionality, it should be documented in a provider documentation.
core/activity/src/api.rs
Outdated
| @@ -14,11 +15,11 @@ pub fn web_scope(db: &DbExecutor) -> Scope { | |||
| /// Common operations for both sides: Provider and Requestor | |||
| mod common { | |||
| use actix_web::{web, Responder}; | |||
| use ya_service_bus::{timeout::IntoTimeoutFuture, RpcEndpoint}; | |||
There was a problem hiding this comment.
It's the only change in this file. What is the benefit of this & how is it related to custom counters?
core/market/src/config.rs
Outdated
| @@ -1,4 +1,5 @@ | |||
| use std::time::Duration; | |||
|
|
|||
golem_cli/src/command/provider.rs
Outdated
| .arg(format!("Init price={}", usage_coeffs.initial)); | ||
| for &(k, v) in usage_coeffs { | ||
| cmd.arg("--price").arg(format!("{}={}", k, v)); | ||
| } |
prekucki
force-pushed
the
feature/custom-counters
branch
from
657fe8b to
0b1f396
Compare
There was a problem hiding this comment.
After clarification from @prekucki that, that this is mostly splitting previous code & after removing changes unrelated to provider - I approve.
For the next PRs I still kindly ask to avoid changes that reorganize whole use sequence only to add/remove one module or adding functionality unrelated to issue subject (vide plugins/ subdir).
Example exe-unit configuration
[ { "name": "gminer", "version": "0.1.0", "supervisor-path": "ya-runtime-gminer", "description": "Golem mining (PoC)", "extra-args": [], "config": { "counters": { "golem.usage.mining.hash": { "name": "share", "description": "Hash share normalized to 1GH", "price": true }, "golem.usage.mining.share": { "name": "raw-share", "description": "Share count", "price": false }, "golem.usage.duration_sec": { "name": "duration", "description": "Duration", "price": true } } } } ]