Fix sink upgrade - error to warning#1132
Fix sink upgrade - error to warning#1132Mohamed-Elfardy wants to merge 7 commits intocybertec-postgresql:masterfrom
Conversation
|
please fix the failing linter test. |
Hi, The workflow is waiting for your approval. |
Pull Request Test Coverage Report for Build 21474436026Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
1cf76b0 to
978bb27
Compare
cmd/pgwatch/version.go
Outdated
| ConfigSchema = "00824" | ||
| SinkSchema = "01110" |
There was a problem hiding this comment.
| ConfigSchema = "00824" | |
| SinkSchema = "01110" | |
| configSchema = "00824" | |
| sinkSchema = "01110" |
I think lower-cased var names are better to keep the whole style of the definition
internal/cmdopts/cmdconfig.go
Outdated
| } else { | ||
| opts.CompleteCommand(ExitCodeConfigError) | ||
| return errors.New("configuration storage does not support upgrade") | ||
| log.GetLogger(ctx).Warn("configuration storage does not support upgrade, skipping") |
There was a problem hiding this comment.
No, we shouldn't log here. We should return error. But let user ability to remove config option completely and re-run the command.
internal/cmdopts/cmdconfig.go
Outdated
There was a problem hiding this comment.
Because here we return error. We must behave the same in all situations.
The problem was not in how we handle, but that it's impossible to run upgrade command without specifying --sources and --metrics flags. This is the main issue! We must allow upgrade specifying only minimal --sink or --metrics options
|
Thanks for the detailed feedback, that makes sense I’ll rework the logic to make the behavior consistent across all cases and update the implementation accordingly. |
|
All checks are passing now. If there are any remaining adjustments you’d like me to make, please let me know and I’ll address them. |
3ecac17 to
bf7ae1c
Compare
|
overruled by #1175. Thanks for your contribution! |
Allow sink-only config upgrades when sources or metrics storage do not support upgrades.
Unsupported upgrades now emit a warning instead of failing, and the sink schema upgrade proceeds as expected.
The "--version" output was also updated to include the sink schema version.
the error message turned into warning.
Fixes #1114