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

feat: Add support for setting the redis username when using redis persistence. #142

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

hexedpackets
Copy link

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

Describe the solution you've provided

eredis supports taking in a username for connections in addition to the password. The default is undefined, which will remain unchanged unless redis_password is explicitly configured.

@hexedpackets hexedpackets requested a review from a team as a code owner January 17, 2025 17:36
@kinyoklion kinyoklion changed the title Allow setting a username for Redis connections feat: Add support for setting the redis username when using redis persistence. Jan 17, 2025
@kinyoklion
Copy link
Member

@hexedpackets Thank you for the contribution! This seems like a reasonable addition. It may take a little bit to review and validate.

@kinyoklion
Copy link
Member

@hexedpackets It appears that some changes will be needed to get the unit tests passing.

@hexedpackets
Copy link
Author

@hexedpackets It appears that some changes will be needed to get the unit tests passing.

Tests are passing but there was a typo in the type spec. I pushed up a fix for that and dialyzer is passing locally for me now.

Side note - might be worth adding an instruction to run make dialyze in the CONTRIBUTING.md document. I didn't realize that it ran separately from the tests task until it failed in CI.

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