Skip to content

Conversation

sazzad16
Copy link
Contributor

@sazzad16 sazzad16 commented Dec 13, 2020

Original author: @yapei123 (#1606)

Closes #1602
Closes #1606

Closes #533
Closes #2245

@sazzad16 sazzad16 marked this pull request as draft December 13, 2020 16:29
@sazzad16 sazzad16 marked this pull request as ready for review December 13, 2020 18:19
@sazzad16 sazzad16 added this to the 3.5.0 milestone Dec 13, 2020
@sazzad16 sazzad16 requested a review from gkorland December 14, 2020 03:39
}

public void setPassword(final String user, final String password) throws IllegalArgumentException {
if (!java.util.Objects.equals(this.user, user)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@sazzad16 why do we force same user?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gkorland In redis-cli on Redis ACL, after doing AUTH user pass once, if we want to AUTH again, we have to do AUTH user newpass not just AUTH newpass. I tried to imitate that here.
I also thought, this would help to us to avoid any mistaken password change as a wrong password (but a valid one for other user) would break the whole pool.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a JavaDoc explaining it

this.factory = factory;

HostAndPort master = initSentinels(sentinels, masterName);
initPool(poolConfig, factory);
Copy link
Contributor

Choose a reason for hiding this comment

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

@sazzad16 perhaps I get it wrong, but do you need to call initPool twice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gkorland initPool(poolConfig, factory); was usually get called within initPool(master); (Line 219). But as we are doing this.factory = factory; (Line 192) now, the if (factory == null) check (Line 216) would not let initPool(poolConfig, factory); to happen at usual place. So we're doing it here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please at least leave a comment in the code explaining this double call.

@sazzad16 sazzad16 force-pushed the reset-password-JedisFactory branch from 076440a to 1de4e0f Compare December 28, 2020 14:12
@sazzad16 sazzad16 modified the milestones: 3.5.0, 3.6.0 Jan 19, 2021
@sazzad16 sazzad16 marked this pull request as draft January 28, 2021 07:29
@sazzad16 sazzad16 deleted the reset-password-JedisFactory branch March 18, 2021 09: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.

Request New Feature: allowing the ability to reset password during runtime for Jedis's auth Make JedisFactory as public class

3 participants