refactor: all lambdas #248
Merged
Conversation
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
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Summary by CodeRabbit
WalkthroughThis PR refactors four Lambda handlers (call-profile, call-profiles, health-check, verify) to use centralized dependency injection through a new Changes
Sequence Diagram(s)sequenceDiagram
participant Main as main()
participant Utils as utils.InitializeLambdaWithFirestore
participant Lambda as lambda.Start
participant Handler as handler()
participant Deps as *utils.Deps
Note over Main,Handler: New Architecture
Main->>Utils: InitializeLambdaWithFirestore("service-name", handlerFunc)
Utils->>Utils: Create background context
Utils->>Utils: Initialize Firestore client
Utils->>Deps: Construct Deps{Client, Ctx}
Utils->>Lambda: Create wrapper & lambda.Start(wrapped)
Lambda->>Handler: Invoke with wrapped request
Handler->>Deps: Access d.Client & d.Ctx
Handler->>Handler: Process request
Handler-->>Lambda: Return response, error
Lambda-->>Main: Complete
sequenceDiagram
participant Handler as handler()
participant WorkerPool as WorkerPool
participant Worker as worker goroutine
participant Job as callProfile/healthCheck
Note over Handler,Job: Concurrency Pattern Change (e.g., call-profiles)
Handler->>WorkerPool: NewWorkerPool(numWorkers, queueSize)
loop For each Firestore doc
Handler->>WorkerPool: Submit(job)
WorkerPool->>WorkerPool: Enqueue job to channel
end
Worker->>Worker: Recover from panic
Worker->>Job: Execute job
Job-->>Worker: Complete/error
Worker->>Worker: Continue polling
Handler->>WorkerPool: Wait()
WorkerPool->>WorkerPool: Close job channel (sync.Once)
WorkerPool->>Worker: Workers drain & exit
Worker-->>WorkerPool: Done
WorkerPool-->>Handler: All jobs complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
 |
Unclear struct name abbreviation ▹ view | |
 |
HTTP client created per request ▹ view | |
|
Context parameter ignored in HTTP request ▹ view | |
|
Misleading Function Name ▹ view | |
|
Duplicate HTTP Request Setup Logic ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| layer/utils/handler.go | ✅ |
| layer/utils/auth.go | ✅ |
| layer/utils/http.go | ✅ |
| layer/utils/concurrency.go | ✅ |
| layer/utils/errors.go | ✅ |
| layer/utils/constants.go | ✅ |
| call-profiles/main.go | ✅ |
| health-check/main.go | ✅ |
| verify/main.go | ✅ |
| layer/utils/logger.go | ✅ |
| call-profile/main.go | ✅ |
| layer/utils/firestore.go | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (17)
call-profile/main.go(4 hunks)call-profile/main_test.go(5 hunks)call-profiles/main.go(4 hunks)call-profiles/main_test.go(1 hunks)health-check/main.go(2 hunks)health-check/main_test.go(2 hunks)layer/utils/auth.go(1 hunks)layer/utils/concurrency.go(1 hunks)layer/utils/constants.go(1 hunks)layer/utils/errors.go(1 hunks)layer/utils/firestore.go(5 hunks)layer/utils/handler.go(1 hunks)layer/utils/http.go(1 hunks)layer/utils/logger.go(1 hunks)verify/helpers_test.go(1 hunks)verify/main.go(4 hunks)verify/main_test.go(10 hunks)
🧰 Additional context used
🧬 Code graph analysis (12)
layer/utils/firestore.go (6)
layer/utils/constants.go (4)
Res(18-31)Constants(63-83)User(55-61)Log(11-16)layer/utils/validation.go (1)
DiffToRes(32-47)layer/utils/logger.go (1)
LogWarnWithError(133-135)layer/utils/http.go (2)
CreateHTTPClient(12-19)DoRequestWithContext(21-23)layer/utils/logging.go (1)
LogProfileSkipped(29-44)layer/utils/errors.go (1)
NewProfileError(36-42)
health-check/main_test.go (1)
layer/utils/handler.go (1)
Deps(12-15)
health-check/main.go (5)
layer/utils/logger.go (1)
GetLogger(117-119)layer/utils/http.go (1)
GetWithContext(43-58)layer/utils/handler.go (2)
Deps(12-15)InitializeLambdaWithFirestore(19-41)layer/utils/concurrency.go (1)
NewWorkerPool(30-50)layer/utils/constants.go (1)
User(55-61)
call-profile/main.go (7)
layer/utils/handler.go (2)
Deps(12-15)InitializeLambdaWithFirestore(19-41)layer/utils/constants.go (2)
User(55-61)Diff(33-49)layer/utils/logging.go (2)
LogProfileSkipped(29-44)LogHealth(46-61)layer/utils/firestore.go (2)
SetProfileStatusBlocked(76-127)Getdata(132-241)layer/utils/http.go (1)
GetWithContext(43-58)layer/utils/validation.go (1)
DiffToRes(32-47)layer/utils/errors.go (2)
ProfileError(19-23)HandleProfileSkippedError(65-70)
layer/utils/handler.go (2)
layer/utils/logger.go (1)
GetLogger(117-119)layer/utils/firestore.go (1)
InitializeFirestoreClient(24-37)
layer/utils/auth.go (1)
layer/utils/logger.go (1)
LogError(137-139)
layer/utils/concurrency.go (1)
layer/utils/logger.go (1)
GetLogger(117-119)
call-profiles/main_test.go (1)
layer/utils/handler.go (1)
Deps(12-15)
verify/main.go (4)
layer/utils/http.go (1)
PostWithContext(25-41)layer/utils/handler.go (2)
Deps(12-15)InitializeLambdaWithFirestore(19-41)layer/utils/firestore.go (3)
GetUserIdFromBody(262-270)GetUserData(276-329)SetProfileStatus(334-354)layer/utils/logging.go (1)
LogVerification(63-88)
verify/main_test.go (1)
layer/utils/handler.go (1)
Deps(12-15)
call-profile/main_test.go (1)
layer/utils/handler.go (1)
Deps(12-15)
call-profiles/main.go (3)
layer/utils/logger.go (1)
GetLogger(117-119)layer/utils/handler.go (2)
Deps(12-15)InitializeLambdaWithFirestore(19-41)layer/utils/concurrency.go (1)
NewWorkerPool(30-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: run_tests
🔇 Additional comments (4)
layer/utils/auth.go (1)
49-52: LGTM: Proper migration to structured logging.The change replaces direct logging with centralized structured error logging, providing better observability with structured fields for function name and parameter name.
verify/helpers_test.go (1)
213-213: LGTM: Test updated to use context-first API.The change correctly passes
context.Background()as the first parameter to theverifyfunction, aligning with the refactored context-aware signature.health-check/main_test.go (1)
400-404: LGTM: Test properly migrated to public Deps API.The test correctly constructs dependencies using the public
utils.Depstype and invokes the handler with the new signaturehandler(d, request). This aligns with the centralized dependency injection pattern.call-profiles/main_test.go (1)
305-309: LGTM: Test properly migrated to public Deps API.The test correctly constructs dependencies using
&utils.Deps{ Client: client, Ctx: ctx }and invokes the handler with the updated signature. Consistent with the broader refactoring effort.
lakshayman
force-pushed
the
refactor-all-lambdas
branch
from
2759f19 to
a9c2ac4
Compare
prakashchoudhary07
approved these changes
Nov 11, 2025
Contributor
prakashchoudhary07
left a comment
prakashchoudhary07
left a comment
There was a problem hiding this comment.
As discussed, the changes will be made in next PR by @lakshayman
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Date: 10/11/2025
Developer Name: @lakshayman
Issue Ticket Number
#247
Description
Improved code quality
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
SCREENSHOT TO SUPPORT NEW LOGGINGTest Coverage
Screenshot 1
Additional Notes