Skip to content

Commit 0066b5d

Browse files
committed
refactor(config)!: name the update wait control wait_mode
UpdateConfigRequest.consistency needed a comment explaining that it is a completion mode, not a database consistency level. Rename the field to wait_mode and the enum to ConfigUpdateWaitMode (CONFIG_UPDATE_WAIT_MODE_COMMIT_ONLY and CONFIG_UPDATE_WAIT_MODE_WAIT_FOR_COMPLETION). It now sits next to wait_timeout and needs no disclaimer. The field number is unchanged and the field was never released. Update the CLI, the Go SDK (ConfigUpdate.WaitMode, ConfigUpdateWaitMode), and the TypeScript client to match. Signed-off-by: Piotr Mlocek <pmlocek@nvidia.com>
1 parent e52edc3 commit 0066b5d

12 files changed

Lines changed: 107 additions & 109 deletions

File tree

‎crates/openshell-cli/src/run.rs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ use openshell_bootstrap::{
4848
use openshell_core::net::set_tcp_nodelay_best_effort;
4949
use openshell_core::proto::{
5050
ApproveAllDraftChunksRequest, ApproveDraftChunkRequest, BeginRootfsTarStagingRequest,
51-
ClearDraftChunksRequest, ConfigUpdateConsistency, ConfigUpdateOperation,
52-
ConfigUpdateOperationState, CreateSandboxRequest, CreateSandboxTemplateRequest,
51+
ClearDraftChunksRequest, ConfigUpdateOperation, ConfigUpdateOperationState,
52+
ConfigUpdateWaitMode, CreateSandboxRequest, CreateSandboxTemplateRequest,
5353
CreateSshSessionRequest, DeleteSandboxRequest, DeleteSandboxTemplateRequest,
5454
DeleteServiceRequest, DeletionOutcome, EndpointResult, EndpointStatus, ExecSandboxRequest,
5555
ExposeServiceRequest, GetCurrentUserRequest, GetDraftHistoryRequest, GetDraftPolicyRequest,
@@ -5699,10 +5699,10 @@ pub async fn sandbox_policy_set(
56995699
workspace.to_string(),
57005700
)),
57015701
policy: Some(policy),
5702-
consistency: if wait {
5703-
ConfigUpdateConsistency::WaitForCompletion.into()
5702+
wait_mode: if wait {
5703+
ConfigUpdateWaitMode::WaitForCompletion.into()
57045704
} else {
5705-
ConfigUpdateConsistency::CommitOnly.into()
5705+
ConfigUpdateWaitMode::CommitOnly.into()
57065706
},
57075707
wait_timeout: Some(
57085708
openshell_core::time::duration_from_std(Duration::from_secs(timeout_secs))
@@ -5833,10 +5833,10 @@ pub async fn sandbox_policy_update(
58335833
workspace.to_string(),
58345834
)),
58355835
merge_operations: plan.merge_operations,
5836-
consistency: if wait {
5837-
ConfigUpdateConsistency::WaitForCompletion.into()
5836+
wait_mode: if wait {
5837+
ConfigUpdateWaitMode::WaitForCompletion.into()
58385838
} else {
5839-
ConfigUpdateConsistency::CommitOnly.into()
5839+
ConfigUpdateWaitMode::CommitOnly.into()
58405840
},
58415841
wait_timeout: Some(
58425842
openshell_core::time::duration_from_std(Duration::from_secs(timeout_secs))

‎crates/openshell-server/src/grpc/mod.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -632,10 +632,10 @@ impl OpenShell for OpenShellService {
632632
&self,
633633
request: Request<UpdateConfigRequest>,
634634
) -> Result<Response<UpdateConfigResponse>, Status> {
635-
let consistency =
636-
openshell_core::proto::ConfigUpdateConsistency::try_from(request.get_ref().consistency)
637-
.map_err(|_| Status::invalid_argument("unknown update consistency"))?;
638-
let wait = consistency == openshell_core::proto::ConfigUpdateConsistency::WaitForCompletion;
635+
let wait_mode =
636+
openshell_core::proto::ConfigUpdateWaitMode::try_from(request.get_ref().wait_mode)
637+
.map_err(|_| Status::invalid_argument("unknown configuration update wait mode"))?;
638+
let wait = wait_mode == openshell_core::proto::ConfigUpdateWaitMode::WaitForCompletion;
639639
if wait && request.get_ref().global {
640640
return Err(Status::invalid_argument(
641641
"WAIT_FOR_COMPLETION is only supported for sandbox-scoped updates",

‎crates/openshell-server/src/grpc/mutation_replay.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ pub(super) fn fingerprint<M: Mutation>(request: &M) -> Result<String, Status> {
482482
message.clear_field_by_name("request_id");
483483
if M::METHOD == "UpdateConfig" {
484484
// Waiting changes response timing, never the admitted mutation identity.
485-
message.clear_field_by_name("consistency");
485+
message.clear_field_by_name("wait_mode");
486486
message.clear_field_by_name("wait_timeout");
487487
}
488488
let value = serde_json::to_value(message)

‎crates/openshell-server/src/grpc/mutation_replay/ordinary/tests.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,7 +1160,7 @@ async fn draft_receipts_replay_after_chunk_state_and_review_tokens_change() {
11601160
async fn config_completion_timeout_preserves_replayable_committed_operation() {
11611161
use openshell_core::proto::open_shell_server::OpenShell;
11621162
use openshell_core::proto::{
1163-
ConfigUpdateConsistency, ConfigUpdateOperationState, Sandbox, SandboxPhase, SandboxSpec,
1163+
ConfigUpdateOperationState, ConfigUpdateWaitMode, Sandbox, SandboxPhase, SandboxSpec,
11641164
SandboxStatus, SettingValue, UpdateConfigRequest, setting_value,
11651165
};
11661166
let (_directory, mut state) = protected_state().await;
@@ -1192,7 +1192,7 @@ async fn config_completion_timeout_preserves_replayable_committed_operation() {
11921192
value: Some(setting_value::Value::BoolValue(true)),
11931193
}),
11941194
request_id: uuid::Uuid::new_v4().to_string(),
1195-
consistency: ConfigUpdateConsistency::WaitForCompletion.into(),
1195+
wait_mode: ConfigUpdateWaitMode::WaitForCompletion.into(),
11961196
wait_timeout: Some(prost_types::Duration {
11971197
seconds: 1,
11981198
nanos: 0,
@@ -1204,7 +1204,7 @@ async fn config_completion_timeout_preserves_replayable_committed_operation() {
12041204
.await
12051205
.unwrap_err();
12061206
assert_eq!(error.code(), Code::DeadlineExceeded, "{error:?}");
1207-
request.consistency = ConfigUpdateConsistency::CommitOnly.into();
1207+
request.wait_mode = ConfigUpdateWaitMode::CommitOnly.into();
12081208
request.wait_timeout = None;
12091209
let response = service
12101210
.update_config(authed_request(request.clone()))
@@ -1247,7 +1247,7 @@ async fn config_completion_timeout_preserves_replayable_committed_operation() {
12471247
.unwrap();
12481248
assert_eq!(pending.len(), 2);
12491249

1250-
request.consistency = i32::MAX;
1250+
request.wait_mode = i32::MAX;
12511251
let error = service
12521252
.update_config(authed_request(request))
12531253
.await

‎crates/openshell-server/src/grpc/policy.rs‎

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ use openshell_core::proto::{
5454
AddAllowRules as ProtoAddAllowRules, AddDenyRules as ProtoAddDenyRules,
5555
ApproveAllDraftChunksRequest, ApproveAllDraftChunksResponse, ApproveDraftChunkRequest,
5656
ApproveDraftChunkResponse, ClearDraftChunksRequest, ClearDraftChunksResponse,
57-
ConfigUpdateConsistency, DraftHistoryEntry, EditDraftChunkRequest, EditDraftChunkResponse,
57+
ConfigUpdateWaitMode, DraftHistoryEntry, EditDraftChunkRequest, EditDraftChunkResponse,
5858
EffectiveSetting, GetConfigUpdateOperationRequest, GetConfigUpdateOperationResponse,
5959
GetDraftHistoryRequest, GetDraftHistoryResponse, GetDraftPolicyRequest, GetDraftPolicyResponse,
6060
GetGatewayConfigRequest, GetGatewayConfigResponse, GetSandboxConfigRequest,
@@ -150,14 +150,14 @@ pub(super) fn config_wait_timeout(
150150
async fn finish_config_update_operation(
151151
state: &Arc<ServerState>,
152152
operation_id: &str,
153-
consistency: ConfigUpdateConsistency,
153+
wait_mode: ConfigUpdateWaitMode,
154154
timeout: std::time::Duration,
155155
) -> Result<Response<UpdateConfigResponse>, Status> {
156156
// Boxed so the many call sites in the update handler stay small.
157157
Box::pin(finish_config_update_operation_inner(
158158
state,
159159
operation_id,
160-
consistency,
160+
wait_mode,
161161
timeout,
162162
))
163163
.await
@@ -166,7 +166,7 @@ async fn finish_config_update_operation(
166166
async fn finish_config_update_operation_inner(
167167
state: &Arc<ServerState>,
168168
operation_id: &str,
169-
consistency: ConfigUpdateConsistency,
169+
wait_mode: ConfigUpdateWaitMode,
170170
timeout: std::time::Duration,
171171
) -> Result<Response<UpdateConfigResponse>, Status> {
172172
if !crate::config_delivery::push_enabled(state) {
@@ -181,7 +181,7 @@ async fn finish_config_update_operation_inner(
181181
return Ok(Response::new(response));
182182
}
183183
config_update_operation::reconcile_one(state, operation_id).await?;
184-
if consistency == ConfigUpdateConsistency::WaitForCompletion {
184+
if wait_mode == ConfigUpdateWaitMode::WaitForCompletion {
185185
config_update_operation::wait_for_terminal(state, operation_id, timeout).await?;
186186
}
187187
let record = config_update_operation::get_record(state, operation_id)
@@ -205,7 +205,7 @@ async fn finish_unchanged_config_update(
205205
workspace: &str,
206206
identity: ConfigOperationIdentity<'_>,
207207
response: Response<UpdateConfigResponse>,
208-
consistency: ConfigUpdateConsistency,
208+
wait_mode: ConfigUpdateWaitMode,
209209
timeout: std::time::Duration,
210210
) -> Result<Response<UpdateConfigResponse>, Status> {
211211
let response = response.into_inner();
@@ -235,7 +235,7 @@ async fn finish_unchanged_config_update(
235235
super::persistence_error_to_status(error, "persist unchanged update operation")
236236
})?;
237237
let operation_id = config_update_operation::public_operation(&record)?.operation_id;
238-
finish_config_update_operation(state, &operation_id, consistency, timeout).await
238+
finish_config_update_operation(state, &operation_id, wait_mode, timeout).await
239239
}
240240

241241
// Private wire-only compatibility types for policy history written before
@@ -3990,9 +3990,9 @@ pub(super) async fn handle_update_config_commit(
39903990
mut request: Request<UpdateConfigRequest>,
39913991
) -> Result<Response<UpdateConfigResponse>, Status> {
39923992
if !request.get_ref().global
3993-
&& request.get_ref().consistency == ConfigUpdateConsistency::WaitForCompletion as i32
3993+
&& request.get_ref().wait_mode == ConfigUpdateWaitMode::WaitForCompletion as i32
39943994
{
3995-
request.get_mut().consistency = ConfigUpdateConsistency::CommitOnly as i32;
3995+
request.get_mut().wait_mode = ConfigUpdateWaitMode::CommitOnly as i32;
39963996
}
39973997
handle_update_config(state, request).await
39983998
}
@@ -4050,12 +4050,12 @@ async fn handle_update_config_inner(
40504050
let replay_facts = super::mutation_replay::ordinary::Facts::from_request(&request);
40514051
let req = request.into_inner();
40524052
let wait_timeout = config_wait_timeout(req.wait_timeout.as_ref())?;
4053-
let consistency = ConfigUpdateConsistency::try_from(req.consistency)
4054-
.map_err(|_| Status::invalid_argument("unknown update consistency"))?;
4055-
let consistency = if consistency == ConfigUpdateConsistency::Unspecified {
4056-
ConfigUpdateConsistency::CommitOnly
4053+
let wait_mode = ConfigUpdateWaitMode::try_from(req.wait_mode)
4054+
.map_err(|_| Status::invalid_argument("unknown configuration update wait mode"))?;
4055+
let wait_mode = if wait_mode == ConfigUpdateWaitMode::Unspecified {
4056+
ConfigUpdateWaitMode::CommitOnly
40574057
} else {
4058-
consistency
4058+
wait_mode
40594059
};
40604060
if req.idempotency_key.len() > MAX_IDEMPOTENCY_KEY_BYTES {
40614061
return Err(Status::invalid_argument(format!(
@@ -4120,7 +4120,7 @@ async fn handle_update_config_inner(
41204120
));
41214121
}
41224122
if req.global {
4123-
if consistency == ConfigUpdateConsistency::WaitForCompletion {
4123+
if wait_mode == ConfigUpdateWaitMode::WaitForCompletion {
41244124
return Err(Status::invalid_argument(
41254125
"WAIT_FOR_COMPLETION is only supported for sandbox-scoped updates",
41264126
));
@@ -4351,7 +4351,7 @@ async fn handle_update_config_inner(
43514351
return finish_config_update_operation(
43524352
state,
43534353
&operation.operation_id,
4354-
consistency,
4354+
wait_mode,
43554355
wait_timeout,
43564356
)
43574357
.await;
@@ -4442,7 +4442,7 @@ async fn handle_update_config_inner(
44424442
return finish_config_update_operation(
44434443
state,
44444444
&operation_id,
4445-
consistency,
4445+
wait_mode,
44464446
wait_timeout,
44474447
)
44484448
.await;
@@ -4471,7 +4471,7 @@ async fn handle_update_config_inner(
44714471
removed,
44724472
response_annotations,
44734473
),
4474-
consistency,
4474+
wait_mode,
44754475
wait_timeout,
44764476
))
44774477
.await;
@@ -4526,7 +4526,7 @@ async fn handle_update_config_inner(
45264526

45274527
drop(config_guard);
45284528
if let Some(operation_id) = operation_id {
4529-
return finish_config_update_operation(state, &operation_id, consistency, wait_timeout)
4529+
return finish_config_update_operation(state, &operation_id, wait_mode, wait_timeout)
45304530
.await;
45314531
}
45324532
response_annotations = persist_update_config_annotations(
@@ -4553,7 +4553,7 @@ async fn handle_update_config_inner(
45534553
false,
45544554
response_annotations,
45554555
),
4556-
consistency,
4556+
wait_mode,
45574557
wait_timeout,
45584558
))
45594559
.await;
@@ -4654,7 +4654,7 @@ async fn handle_update_config_inner(
46544654
emit_config_update_policy_success(sandbox_caller);
46554655

46564656
if let Some(operation_id) = operation_id {
4657-
return finish_config_update_operation(state, &operation_id, consistency, wait_timeout)
4657+
return finish_config_update_operation(state, &operation_id, wait_mode, wait_timeout)
46584658
.await;
46594659
}
46604660
return Box::pin(finish_unchanged_config_update(
@@ -4673,7 +4673,7 @@ async fn handle_update_config_inner(
46734673
false,
46744674
response_annotations,
46754675
),
4676-
consistency,
4676+
wait_mode,
46774677
wait_timeout,
46784678
))
46794679
.await;
@@ -4804,7 +4804,7 @@ async fn handle_update_config_inner(
48044804
false,
48054805
response_annotations,
48064806
),
4807-
consistency,
4807+
wait_mode,
48084808
wait_timeout,
48094809
))
48104810
.await;
@@ -4901,7 +4901,7 @@ async fn handle_update_config_inner(
49014901
spawn_pending_chunk_refresh(state, &workspace, &sandbox);
49024902
emit_full_policy_update_success(sandbox_caller, next_version);
49034903

4904-
finish_config_update_operation(state, &operation_id, consistency, wait_timeout).await
4904+
finish_config_update_operation(state, &operation_id, wait_mode, wait_timeout).await
49054905
}
49064906

49074907
// ---------------------------------------------------------------------------
@@ -13321,7 +13321,7 @@ mod tests {
1332113321
value: Some(setting_value::Value::BoolValue(true)),
1332213322
}),
1332313323
workspace_scope: Some(openshell_core::proto::workspace_selector("default")),
13324-
consistency: ConfigUpdateConsistency::WaitForCompletion.into(),
13324+
wait_mode: ConfigUpdateWaitMode::WaitForCompletion.into(),
1332513325
idempotency_key: "poll-wait".to_string(),
1332613326
..Default::default()
1332713327
})),
@@ -13351,7 +13351,7 @@ mod tests {
1335113351
value: Some(setting_value::Value::BoolValue(true)),
1335213352
}),
1335313353
workspace_scope: Some(openshell_core::proto::workspace_selector("default")),
13354-
consistency: ConfigUpdateConsistency::WaitForCompletion.into(),
13354+
wait_mode: ConfigUpdateWaitMode::WaitForCompletion.into(),
1335513355
wait_timeout: openshell_core::time::duration_from_std(std::time::Duration::from_secs(
1335613356
5,
1335713357
))
@@ -13396,7 +13396,7 @@ mod tests {
1339613396
);
1339713397
state.store.put_message(&sandbox).await.unwrap();
1339813398
// A supervisor without snapshot support is never registered with the
13399-
// delivery scheduler.
13399+
// delivery queue.
1340013400
let (tx, _rx) = tokio::sync::mpsc::channel(1);
1340113401
state.supervisor_sessions.register(
1340213402
"sb-legacy-operation".into(),
@@ -13456,7 +13456,7 @@ mod tests {
1345613456
value: Some(setting_value::Value::BoolValue(true)),
1345713457
}),
1345813458
workspace_scope: Some(openshell_core::proto::workspace_selector("default")),
13459-
consistency: ConfigUpdateConsistency::WaitForCompletion.into(),
13459+
wait_mode: ConfigUpdateWaitMode::WaitForCompletion.into(),
1346013460
wait_timeout: openshell_core::time::duration_from_std(
1346113461
std::time::Duration::from_secs(5),
1346213462
)
@@ -23153,7 +23153,7 @@ mod tests {
2315323153
expected_resource_version: current_version,
2315423154
annotations: HashMap::new(),
2315523155

23156-
consistency: ConfigUpdateConsistency::CommitOnly.into(),
23156+
wait_mode: ConfigUpdateWaitMode::CommitOnly.into(),
2315723157
idempotency_key: String::new(),
2315823158
wait_timeout: None,
2315923159
}),
@@ -23256,7 +23256,7 @@ mod tests {
2325623256
expected_resource_version: current_version,
2325723257
annotations: annotations.clone(),
2325823258

23259-
consistency: ConfigUpdateConsistency::CommitOnly.into(),
23259+
wait_mode: ConfigUpdateWaitMode::CommitOnly.into(),
2326023260
idempotency_key: String::new(),
2326123261
wait_timeout: None,
2326223262
}),
@@ -24314,7 +24314,7 @@ mod tests {
2431424314
expected_resource_version: 99, // stale version
2431524315
annotations: HashMap::new(),
2431624316

24317-
consistency: ConfigUpdateConsistency::CommitOnly.into(),
24317+
wait_mode: ConfigUpdateWaitMode::CommitOnly.into(),
2431824318
idempotency_key: String::new(),
2431924319
wait_timeout: None,
2432024320
}),
@@ -24420,7 +24420,7 @@ mod tests {
2442024420
expected_resource_version: initial_version,
2442124421
annotations: HashMap::new(),
2442224422

24423-
consistency: ConfigUpdateConsistency::CommitOnly.into(),
24423+
wait_mode: ConfigUpdateWaitMode::CommitOnly.into(),
2442424424
idempotency_key: String::new(),
2442524425
wait_timeout: None,
2442624426
}),
@@ -24822,7 +24822,7 @@ mod tests {
2482224822
value: Some(setting_value::Value::BoolValue(true)),
2482324823
}),
2482424824
workspace_scope: Some(openshell_core::proto::workspace_selector("default")),
24825-
consistency: ConfigUpdateConsistency::WaitForCompletion.into(),
24825+
wait_mode: ConfigUpdateWaitMode::WaitForCompletion.into(),
2482624826
idempotency_key: "inactive-setting-1".to_string(),
2482724827
expected_resource_version: 0,
2482824828
annotations: HashMap::from([("change-ticket".to_string(), "1234".to_string())]),
@@ -24928,7 +24928,7 @@ mod tests {
2492824928
value: Some(setting_value::Value::BoolValue(true)),
2492924929
}),
2493024930
workspace_scope: Some(openshell_core::proto::workspace_selector("default")),
24931-
consistency: ConfigUpdateConsistency::CommitOnly.into(),
24931+
wait_mode: ConfigUpdateWaitMode::CommitOnly.into(),
2493224932
idempotency_key: "wake-local-waiter".to_string(),
2493324933
..Default::default()
2493424934
})),
@@ -25021,7 +25021,7 @@ mod tests {
2502125021
value: Some(setting_value::Value::BoolValue(true)),
2502225022
}),
2502325023
workspace_scope: Some(openshell_core::proto::workspace_selector("default")),
25024-
consistency: ConfigUpdateConsistency::CommitOnly.into(),
25024+
wait_mode: ConfigUpdateWaitMode::CommitOnly.into(),
2502525025
idempotency_key: "poll-missed-notification".to_string(),
2502625026
..Default::default()
2502725027
})),

‎crates/openshell-server/src/storage_proto.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ mod tests {
234234
// SessionRedirect and SupervisorHello.redirected are supervisor control
235235
// traffic and are never stored.
236236
const PUBLIC_RPC_SCHEMA_SHA256: &str =
237-
"8ad3b7b06720a458ad497701ce96531fafb01c4ba622a6f9b937e00b206e1fec";
237+
"c628cc935f0f8b41a9bfeea2342cde6f4e0da9a58bdaf96d533606ff3d6309a7";
238238
const DURABLE_SCHEMA_SHA256: &str =
239239
"947396e9e9f73b8e2971e923137eea4b9d5bdc4f67d88db6237c358756ffd8a8";
240240
const PUBLIC_DURABLE_OVERLAP_SHA256: &str =

0 commit comments

Comments
 (0)