-
Notifications
You must be signed in to change notification settings - Fork 21
Improve already-exists handling and add e2e coverage #370
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@freeznet:Thanks for your contribution. For this PR, do we need to update docs? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
license-eye has totally checked 455 files.
| Valid | Invalid | Ignored | Fixed |
|---|---|---|---|
| 363 | 1 | 91 | 0 |
Click to see the invalid file list
- pkg/admin/errors_test.go
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR improves error handling for already-existing Pulsar resources and adds end-to-end test coverage for pre-existing topics and schema version stability. The changes prevent unnecessary reconcile failures and schema version churn.
Changes:
- Enhanced
IsAlreadyExisterror detection to handle multiple error formats and HTTP status codes - Refactored schema application logic to compare versions and avoid unnecessary uploads
- Added comprehensive unit and e2e tests for pre-existing topics and empty schema handling
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/admin/errors.go | Improved IsAlreadyExist to handle wrapped errors and string matching for "already exist" messages |
| pkg/admin/errors_test.go | Added comprehensive test coverage for IsAlreadyExist function |
| pkg/admin/interface.go | Added GetSchemaInfoWithVersion and GetSchemaVersionBySchemaInfo methods to the interface |
| pkg/admin/impl.go | Implemented new schema version checking methods |
| pkg/admin/dummy.go | Added stub implementations for testing |
| pkg/connection/reconcile_topic.go | Refactored applySchema to use version comparison instead of deep equality |
| pkg/connection/reconcile_topic_test.go | Added unit tests for schema application logic |
| tests/operator/resources_test.go | Added e2e tests for pre-existing topics and empty schema version stability |
| .gitignore | Added mise.toml to ignored files |
Comments suppressed due to low confidence (1)
pkg/admin/errors_test.go:1
- The copyright header is duplicated at the beginning of the file. Remove the duplicate copyright header (lines 15-27).
// Copyright 2026 StreamNative
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes #368
Motivation
Avoid repeated reconcile failures when topics already exist and prevent schema updates from churning on empty schema.
Modifications
Improve already-exists detection and add e2e coverage for pre-existing topics and empty schema version stability.
Verifying this change
This change added tests and can be verified as follows:
go test ./pkg/admin/... -v -run TestIsAlreadyExistcd tests && ginkgo --trace --progress ./operatorDocumentation