feat(host,tasks): complete aggregation api #387
Conversation
Write in-memory implementation Stub out sqlite implementation
petarvujovic98
temporarily deployed
to
test-environment
GitHub Actions
Inactive
petarvujovic98
temporarily deployed
to
test-environment
GitHub Actions
Inactive
| if aggregation_request.proofs.is_empty() { | ||
| return Err(anyhow::anyhow!("No proofs provided").into()); | ||
| } |
There was a problem hiding this comment.
IMO, we don't have to check the proofs.
| ) | ||
| )] | ||
| #[debug_handler(state = ProverState)] | ||
| /// Prune all aggregation tasks. |
There was a problem hiding this comment.
The Prone API is oriented to normal users?
I'm afraid that the Prune API is too dangerous as it poses significant risks. I propose categorizing such APIs under 'admin' for better control if we really need.
IMO, In practice, operations team members should and can perform these actions manually. So, is this API truly necessary?
| let proof_type = ProofType::from_str( | ||
| aggregation_request | ||
| .proof_type | ||
| .as_deref() | ||
| .unwrap_or_default(), | ||
| )?; | ||
| inc_host_req_count(0); | ||
| inc_guest_req_count(&proof_type, 0); |
There was a problem hiding this comment.
To maintain consistency with the 'cancel' API across versions 1 and 2, I recommend removing these two lines of code, as we don't record metrics for 'cancel' API (at least not for now, maybe we can add some metrics for 'cancel' API)
|
|
||
| prover_state | ||
| .task_channel | ||
| .try_send(Message::CancelAggregate(aggregation_request.clone()))?; |
There was a problem hiding this comment.
The cancelling aggregation request may be non-existing. In order to prevent useless 'cancel' requests, we should check the status of the target aggregation request by get_aggregation_task_status() before sending signal to task_channel.
No description provided.