Pull Request#214
Open
aditisb1212-lab wants to merge 15 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR migrates the database layer from Postgres to MySQL by introducing a ChangesMySQL Database Configuration and Health
Sequence Diagram(s)sequenceDiagram
participant App
participant LoadDBConfig
participant InitDatabase
participant GORM
participant MySQL
App->>LoadDBConfig: read env vars
LoadDBConfig-->>App: DBConfig (validated)
App->>InitDatabase: InitDatabase(DBConfig)
InitDatabase->>GORM: gorm.Open(mysql DSN)
GORM->>MySQL: TCP connect
MySQL-->>GORM: connection
InitDatabase->>GORM: configure pool (open/idle/lifetime/idle-time)
InitDatabase->>MySQL: Ping()
MySQL-->>InitDatabase: pong
InitDatabase-->>App: nil (global DB set)
rect rgba(100, 180, 255, 0.5)
App->>HealthCheckDatabase: check
HealthCheckDatabase->>MySQL: Ping()
HealthCheckDatabase-->>App: healthy/unhealthy + stats
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Contributor
|
All contributors have signed the CLA. ✅ Thank you! |
Author
I have read the CLA and agree to its terms |
Author
|
I have read the CLA and agree to its terms |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.env.example:
- Line 70: The FORGOT_RATE_LIMIT_WINDOW environment variable on line 70 contains
an inline comment `//ms` that is being parsed as part of the numeric value,
causing integer parsing with strconv.Atoi to fail. Remove the inline `//ms`
comment from the FORGOT_RATE_LIMIT_WINDOW value so that only the numeric value
`3600000` remains, or if documentation is needed, add the comment on a separate
line outside the variable assignment to prevent it from interfering with the
integer parsing logic.
In `@internal/config/config.go`:
- Around line 139-145: The getEnv function is declared twice in the same package
(at lines 140-145 and 274-279), which causes a Go compilation error since
duplicate function declarations are not allowed. Identify both getEnv function
declarations in the file, determine which implementation is correct for your use
case (the first uses os.LookupEnv to distinguish between missing and
empty-valued variables, while the second uses os.Getenv which treats them
identically), keep the correct implementation, and delete the duplicate
declaration.
In `@internal/config/database.go`:
- Around line 15-16: The InitDatabase function signature returns only error, but
the calling code in main() at line 46 assigns the return value to a database
connection variable (db) and then passes it to config.AutoMigrate at line 59.
Modify the InitDatabase function signature to return both the database
connection and an error value (e.g., returning a tuple or struct containing the
connection alongside the error), then update the function implementation to
properly initialize and return the database connection object along with any
initialization errors.
- Around line 128-133: The capacity health threshold at line 128 is hardcoded to
95, assuming a fixed 100-connection pool, but this does not account for the
actual MaxOpenConns value from cfg.Database.PoolMax configured at line 50. Since
sql.DBStats does not expose MaxOpenConnections, store the configured
MaxOpenConns value in a package-scoped variable (initialized when the database
is configured) or pass it as a parameter to HealthCheckDatabase(). Then replace
the hardcoded threshold of 95 with a dynamic calculation that computes the
threshold as a percentage of the actual configured pool size, so the health
check in the condition at line 128 accurately reflects the actual capacity
limits.
- Around line 93-94: The code at lines 93-94 and 116-117 ignores the error
returned by DB.DB(), which can be nil if the method fails. This causes a panic
when calling Stats() and Ping() on a nil sqlDB. Check the error returned by
DB.DB() and handle it appropriately (such as logging the error and returning
early) before attempting to call Stats() or Ping() on the sqlDB object. Apply
this error handling fix to both occurrences in the file.
- Around line 41-80: In the InitDatabase function that accepts dbConfig as a
parameter, replace all references to cfg.Database with dbConfig throughout the
connection pool configuration section. Additionally, replace the log.Fatal call
with a proper error return statement using fmt.Errorf to handle the ping failure
appropriately. Finally, add the missing SetMaxOpenConns and SetMaxIdleConns
configuration calls to the corrected configuration block along with their
corresponding log statements to ensure all pool settings are consistently
applied.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b5843e28-c1a1-4f39-8cf6-463b4647edf0
📒 Files selected for processing (4)
.env.example.github/pull_request_template.mdinternal/config/config.gointernal/config/database.go
Owner
|
resolev all the issues marked by sonar and coderabbit |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/config/database.go`:
- Line 29: In the InitDatabase function, the Logger field configuration
references an undefined variable logLevel that does not exist as a function
parameter, package-level variable, or in the codebase. Replace the logLevel
variable in the LogMode method call with an appropriate logger level constant
such as logger.Info, logger.Warn, or logger.Error depending on the desired
logging verbosity for the database connection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 755b4e46-bbde-4d40-baac-8dfd83729486
📒 Files selected for processing (1)
internal/config/database.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.env.example:
- Line 70: The FORGOT_RATE_LIMIT_WINDOW environment variable assignment has
trailing whitespace after the value 3600000 that should be removed to comply
with style conventions and prevent unnecessary diffs. Simply delete the
whitespace characters at the end of the line after the numeric value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6959ac0b-9210-425e-b686-6fdc9f88fba9
📒 Files selected for processing (3)
.env.exampleinternal/config/config.gointernal/config/database.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/config/config.go
- internal/config/database.go
|
Author
|
@roshankumar0036singh I have submitted the commits. Please review and merge when you have a moment! |
Author
|
Sir please check again and review i have done all |
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.
Checklist
I have read the CLA and agree to its terms.on this PR.Summary by CodeRabbit
FORGOT_RATE_LIMIT_WINDOWvalue formatting by removing the//mssuffix.Closes<#187>