Update license headers / Format codes #2723
Conversation
Signed-off-by: Vdaas CI <[email protected]>
Deploying vald with
|
| Latest commit: |
a30c357
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://28177ec0.vald.pages.dev |
| Branch Preview URL: | https://formatter-go-and-yaml-format-y9zi.vald.pages.dev |
📝 WalkthroughWalkthroughThis pull request introduces several changes, primarily focusing on the addition of new functionality for deletion operations within the service layer. It includes the creation of two new files: Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
pkg/index/job/deletion/service/options.go (4)
Line range hint
22-31: Consider enhancing error message for nil discovererWhile the nil check is good, the error message could be more descriptive to help with troubleshooting.
Consider updating the error message:
func WithDiscoverer(client discoverer.Client) Option { return func(idx *index) error { if client == nil { - return errors.NewErrCriticalOption("discoverer", client) + return errors.NewErrCriticalOption("discoverer client cannot be nil", client) } idx.client = client return nil } }
Line range hint
33-42: Consider adding upper bound for concurrencyWhile the function correctly validates the lower bound, consider adding an upper bound to prevent potential resource exhaustion.
Consider updating the validation:
func WithIndexingConcurrency(num int) Option { return func(idx *index) error { - if num <= 0 { + const maxConcurrency = 1000 // adjust based on your system's capacity + if num <= 0 || num > maxConcurrency { return errors.NewErrInvalidOption("indexingConcurrency", num) } idx.concurrency = num return nil } }
Line range hint
44-52: Add validation for target addressesThe function should validate each address to ensure they are well-formed and non-empty.
Consider adding validation:
func WithTargetAddrs(addrs ...string) Option { return func(idx *index) error { if len(addrs) != 0 { + for _, addr := range addrs { + if addr == "" { + return errors.NewErrInvalidOption("target address cannot be empty", addr) + } + // Add additional address format validation if needed + } idx.targetAddrs = append(idx.targetAddrs, addrs...) } return nil } }
Line range hint
54-60: Add validation for target index IDThe function should validate that the indexID is non-empty and meets any format requirements.
Consider adding validation:
func WithTargetIndexID(indexID string) Option { return func(idx *index) error { + if indexID == "" { + return errors.NewErrInvalidOption("target index ID cannot be empty", indexID) + } idx.targetIndexID = indexID return nil } }pkg/index/job/deletion/service/deleter.go (2)
Line range hint
52-53: Fix improper usage oferrors.AsfunctionIn the
Newfunction, the usage oferrors.As(oerr, &e)is incorrect becauseeis already a pointer toerrors.ErrCriticalOption. Passing&eresults in a double pointer**errors.ErrCriticalOption, which doesn't match the expected type. The correct usage is to passedirectly without the address operator.Apply this diff to fix the issue:
e := &errors.ErrCriticalOption{} -if errors.As(oerr, &e) { +if errors.As(oerr, e) {
Line range hint
114-212: Consider logging when no target addresses are foundIn the
doDeleteIndexmethod, iftargetAddrsis empty, the function proceeds without logging or returning an error. This could lead to silent failures. Consider adding a check for emptytargetAddrsand logging a warning or returning an error.Apply this diff to add a check for empty
targetAddrs:func (idx *index) doDeleteIndex( ctx context.Context, fn func(_ context.Context, _ vald.RemoveClient, _ ...grpc.CallOption) (*payload.Object_Location, error), ) (errs error) { ctx, span := trace.StartSpan(grpc.WrapGRPCMethod(ctx, grpcMethodName), apiName+"/service/index.doDeleteIndex") defer func() { if span != nil { span.End() } }() + if len(targetAddrs) == 0 { + log.Warn("No target addresses found") + return errors.New("no target addresses found") + } + var emu sync.Mutex
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- .github/workflows/dockers-index-deletion-image.yaml (1 hunks)
- pkg/index/job/deletion/service/deleter.go (1 hunks)
- pkg/index/job/deletion/service/options.go (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/dockers-index-deletion-image.yaml
🔇 Additional comments (4)
pkg/index/job/deletion/service/options.go (1)
1-13: LGTM: License header is properly updatedThe Apache 2.0 license header is correctly formatted and includes the updated copyright year range (2019-2024).
pkg/index/job/deletion/service/deleter.go (3)
Line range hint
76-78: LGTMThe
StartClientmethod correctly starts the gRPC client by callingidx.client.Start(ctx).
Line range hint
80-112: LGTMThe
Startmethod appropriately initiates the deletion process with comprehensive error handling and tracing.
Line range hint
63-73: LGTMThe
delDuplicateAddrsfunction efficiently removes duplicate addresses using a map to track existing entries.
|
[CHATOPS:HELP] ChatOps commands.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2723 +/- ##
==========================================
- Coverage 23.95% 23.95% -0.01%
==========================================
Files 545 545
Lines 54324 54324
==========================================
- Hits 13013 13011 -2
- Misses 40535 40537 +2
Partials 776 776 ☔ View full report in Codecov by Sentry. |
kpango
deleted the
formatter/go_and_yaml/format_at_20241026_155747_597
branch
Signed-off-by: Vdaas CI <[email protected]>
Signed-off-by: Vdaas CI <[email protected]>
Update license headers / Format Go codes and YAML files.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation