Uh oh!
There was an error while loading. Please reload this page.
branch-4.1: [feat](snapshot) Support storage vault for clone instance #63217 - #63533
Conversation
wyxxxcat
commented
May 22, 2026
run buildall |
hello-stephen
commented
May 22, 2026
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
wyxxxcat
commented
May 26, 2026
/review |
There was a problem hiding this comment.
Requesting changes due to post-commit cascade error handling regressions. The PR commits the root update before processing derived instances, but several new/changed paths then return an RPC error if a derived update fails. That exposes partially committed state as a failed operation and makes retries unreliable.
Critical checkpoints:
- Correctness: blocking issues found in cascade update error semantics.
- Data consistency: root/early descendant writes can be committed while later descendant failures are reported to the caller as overall failure.
- Idempotency/retry safety: retrying after these errors can fail or skip repair because the root mutation already happened.
- Tests: added happy-path storage vault cascade coverage, but no failure/retry tests for post-root-commit cascade errors.
- Repository review guide: no changed code under the Nereids expressions guide; its checkLegalityBeforeTypeCoercion rule is not applicable.
- User focus: no additional user-provided review focus was supplied.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
wyxxxcat
commented
May 27, 2026
run buildall |
wyxxxcat
commented
May 27, 2026
/review |
There was a problem hiding this comment.
Review result: request changes.
Critical checkpoints:
- Existing review threads were checked; I did not repeat the already-raised post-commit cascade failure concerns in alter_storage_vault/update_ak_sk.
- Repository AGENTS.md checkpoint: no Nereids expression code is touched, so the checkLegalityBeforeTypeCoercion rule is not applicable.
- User focus: no additional user-provided review focus was specified.
I found one additional blocking regression in the object-store credential update helper: role-based updates now drop the requested cred_provider_type and always store INSTANCE_PROFILE, which changes behavior for non-default providers and can also make same-role provider changes a no-op.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
pick: #63217