Skip to content

Conversation

@itschip
Copy link
Member

@itschip itschip commented Jun 5, 2025

The application usually starts before the db is ready to accept connections, so we retry incase the app is fast af

The application usually starts before the db is ready to accept
connections, so we retry incase the app is fast af
Copilot AI review requested due to automatic review settings June 5, 2025 13:22
Copy link

Copilot AI left a 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 introduces a retry mechanism for connecting to PostgreSQL and MySQL databases to handle scenarios when the database isn’t ready at application startup.

  • Adds retry loops for database connection attempts
  • Integrates additional logging using otelzap alongside existing logging routines

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
internal/database/pg.go Implements retry logic for PostgreSQL connection with enhanced logging.
internal/database/mysql.go Implements retry logic for MySQL connection and updates error messaging.

maxRetries := 3
retryWaitTime := 5 * time.Second

for i := range maxRetries {
Copy link

Copilot AI Jun 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The for-range loop is incorrectly used here because maxRetries is an integer rather than an iterable. Consider using a standard loop such as 'for i := 0; i < maxRetries; i++' for proper iteration.

Suggested change
for i := range maxRetries {
for i := 0; i < maxRetries; i++ {

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong

Comment on lines 25 to 27
if err != nil {
otelzap.L().Fatal("Failed to prepare PostgreSQL connection", zap.Error(err))
}
Copy link

Copilot AI Jun 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error check after opening the database connection is ineffective because 'err' is not updated from sql.OpenDB; consider removing this check or revising the error handling logic.

Suggested change
if err != nil {
otelzap.L().Fatal("Failed to prepare PostgreSQL connection", zap.Error(err))
}

Copilot uses AI. Check for mistakes.
maxRetries := 3
retryWaitTime := 5 * time.Second

for i := range maxRetries {
Copy link

Copilot AI Jun 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The for-range loop is used on an integer (maxRetries) which is invalid; change this to a conventional loop like 'for i := 0; i < maxRetries; i++' to correctly iterate.

Suggested change
for i := range maxRetries {
for i := 0; i < maxRetries; i++ {

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong

for i := range maxRetries {
sqldb, err = sql.Open("mysql", dsn)
if err != nil {
log.Fatalf("failed to prepare mysql connection: %v", err)
Copy link

Copilot AI Jun 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The code mixes the standard log package with otelzap for logging errors; consider using a consistent logging approach throughout the file.

Suggested change
log.Fatalf("failed to prepare mysql connection: %v", err)
otelzap.L().Fatal("Failed to prepare MySQL connection", zap.Error(err))

Copilot uses AI. Check for mistakes.
@itschip itschip merged commit 5f34632 into develop Jun 5, 2025
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants