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

Constructors that accept a URI and a JedisClientConfig pull elements from both without documentation #3982

Open
joshrotenberg opened this issue Sep 30, 2024 · 3 comments

Comments

@joshrotenberg
Copy link

joshrotenberg commented Sep 30, 2024

UnifiedJedis and possibly other type constructors that accept a URI and a JedisClientConfig instantiate themselves with a mix of elements from both. This isn't unreasonable given that the URI can contain more than just host and port, but it isn't documented.

Options to fix it:

  • Document the current behavior
  • Update the constructors to let, for example, any config options win if both are specified, and document the behavior.

For example:

// works
UnifiedJedis jedis = new UnifiedJedis(new HostAndPort("localhost", 6379), DefaultJedisClientConfig.builder().user("username").password("password").build());

// also works
UnifiedJedis jedis2 = new UnifiedJedis(URI.create("redis://username:password@localhost:6379"));

// doesn't set auth correctly
UnifiedJedis ohNo = new UnifiedJedis(URI.create("redis://localhost:6379"), DefaultJedisClientConfig.builder().user("username").password("password").build());

See

.user(JedisURIHelper.getUser(uri)).password(JedisURIHelper.getPassword(uri))

@sazzad16
Copy link
Collaborator

sazzad16 commented Oct 1, 2024

@joshrotenberg Thank you for sharing. Our URI support is not so dynamic yet. In the current design, when you're using URI you should put all parameters that our URI processor supports.

I agree that stronger documentation would be helpful.

@kachida
Copy link

kachida commented Oct 3, 2024

Hi @sazzad16 , @joshrotenberg , please assign this to me, Thanks

@sazzad16
Copy link
Collaborator

sazzad16 commented Oct 3, 2024

@kachida You can craft a PR. No 'assign'ment necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants