feat(config,server,cli,watch): support global general.callback_url (#1481) - #1482
Open
prafful1234 wants to merge 7 commits into
Open
feat(config,server,cli,watch): support global general.callback_url (#1481)#1482prafful1234 wants to merge 7 commits into
prafful1234 wants to merge 7 commits into
Conversation
Provide a shared callback dispatcher with config fallback so one-shot CLI commands and each watch iteration can notify external systems without changing the existing API callback JSON contract.
ValidateConfig now errors on durations <= 0 (e.g: 0s) CallbackTimeoutDuration is always positive after a successful load
API server already notifies via pkg/server dispatchCLICallback now returns early when --command-id is set Also extract applyCLICallbacks add allowlist coverage, and drop unused GetOperationId
`startWatchIteration` no longer registers a `status.Current` row per cycle (AsyncStatus.commands is append-only) preventing unbounded memory growth in long-running watch processes The iteration `operation_id` is now a bare UUID and finish is made idempotent via sync Once the top-level watch commandId is passed to `CreateToRemote/Rebase` again
`parseCallback` now builds each POST from `context.WithoutCancel` plus `general.callback_timeout` Canceled caller context cannot kill the notification and callbacks no longer hang without a deadline
Signed-off-by: slach <bloodjazman@gmail.com>
…into feature/general-callback-url
Author
|
@Slach the failing Test (21.8–26.3) jobs look fixed on master (object-disk streaming fallback). please approve the CI? Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1481
This PR introduces
general.callback_urlandgeneral.callback_timeoutconfiguration options to enable backup completion webhooks across all execution paths: API, CLI, and Watch Mode.Currently, completion callbacks are only available via the API query parameter (
POST /backup/create?callback=...). This PR unifies callback handling into a sharedpkg/statusdispatcher while preserving strict backward compatibility for existing API consumers.Changes
pkg/config): Addedgeneral.callback_urlandgeneral.callback_timeout(default5s), with support forCALLBACK_URLandCALLBACK_TIMEOUTenvironment variables.pkg/status): CreatedSendCallback()supportingCallbackPayload(status,error,operation_id, and optionalcommand/duration).pkg/server): Updated API handler to fall back togeneral.callback_urlif the?callback=query parameter is missing or empty. Query params still take precedence when provided.cmd/clickhouse-backup): Added synchronous callback dispatch in CLI lifecycle hooks, reusing existingcommandId/operation_idtracking. Errors during dispatch are logged and swallowed so CLI exit codes are never affected by callback failures.pkg/backup/watch.go): Added per-iteration callback dispatch andstatus.Current.Start()/Stop()tracking inside the watch loop, ensuring each individual watch cycle sends a clean callback with its own uniqueoperation_id.ReadMe.mdandManual.md, and added an entry toChangeLog.md.Key Constraints Respected
CallbackPayload.Errorintentionally retains noomitemptytag so API callback JSON payloads remain byte-identical ("error": ""is always present).commandanddurationuseomitempty.Testing
Followed workflow across all modified packages:
pkg/config: Tested parsing from YAML, ENV overrides, and default timeout handling.pkg/status: Tested payload serialization, HTTP timeout enforcement, and non-200 response handling.pkg/server: Tested fallback logic when?callback=is absent/empty vs explicitly set.cmd/clickhouse-backup: Tested CLI callback dispatch on command success/failure and confirmed non-zero callback failures do not mutate command exit codes.pkg/backup: Tested per-iteration callback firing and error recovery across multiple watch ticks.All tests pass cleanly:
go test -v ./pkg/... ./cmd/...