Skip to content
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

Add redis as an option for database #177

Merged
merged 8 commits into from
Feb 7, 2024

Conversation

schoolboybru
Copy link
Contributor

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

Problem/Feature

Adding Redis as an option for a database.

Description of Changes:

  1. Add all templates for redis
  2. Make redis an option when selecting a database
  3. Update documentation

Checklist

  • I have self-reviewed the changes being requested
  • I have updated the documentation (if applicable)

Comment on lines 49 to 62
func (s *service) Health() map[string]string {
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
defer cancel()

err := s.db.Ping(ctx)

if err != nil {
log.Fatalf(fmt.Sprintf("db down: %v", err))
}

return map[string]string{
"message": "It's healthy",
}
}
Copy link
Collaborator

@Ujstor Ujstor Feb 6, 2024

Choose a reason for hiding this comment

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

Ping is returning an error, and it is breaking the app when you curl the health route.

I think this solves the issue

func (s *service) Health() map[string]string {
	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
	defer cancel()

	result, err := s.db.Ping(ctx).Result()

	if err != nil {
		log.Fatalf(fmt.Sprintf("db down: %v", err))
	}

	 if result != "PONG" {
        log.Fatalf(fmt.Sprintf("Unexpected ping response: %s", result))
    }

	return map[string]string{
		"message": "It's healthy",
	}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh good catch 👍

Copy link
Collaborator

@Ujstor Ujstor left a comment

Choose a reason for hiding this comment

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

After fixing the Health func, this is ready for merge

@schoolboybru schoolboybru requested a review from Ujstor February 7, 2024 01:31
Copy link
Collaborator

@Ujstor Ujstor left a comment

Choose a reason for hiding this comment

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

LGTM

@Ujstor Ujstor merged commit 2c56ad0 into Melkeydev:main Feb 7, 2024
44 checks passed
@schoolboybru schoolboybru deleted the feat/redis-support branch February 7, 2024 19:18
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