Skip to content
This repository was archived by the owner on Dec 13, 2023. It is now read-only.

Comments

Redis cluster ssl#1216

Closed
bharadwajrembar wants to merge 2 commits intoNetflix:devfrom
bharadwajrembar:redis-cluster-ssl
Closed

Redis cluster ssl#1216
bharadwajrembar wants to merge 2 commits intoNetflix:devfrom
bharadwajrembar:redis-cluster-ssl

Conversation

@bharadwajrembar
Copy link

Fixes #972 , the password is not propagated to the JedisClusterClient and thus, throws an error stating NOAUTH when trying to connect to a Redis instance with Authentication enabled. This PR fixes that. Please review. @kishorebanala @apanicker-nflx , cc: @skyrocknroll

@codecov
Copy link

codecov bot commented Jul 18, 2019

Codecov Report

Merging #1216 into dev will increase coverage by 0.03%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev    #1216      +/-   ##
============================================
+ Coverage     63.61%   63.65%   +0.03%     
- Complexity     2681     2703      +22     
============================================
  Files           236      236              
  Lines         13451    13547      +96     
  Branches       1340     1348       +8     
============================================
+ Hits           8557     8623      +66     
- Misses         4165     4188      +23     
- Partials        729      736       +7     
Impacted Files Coverage Δ Complexity Δ
...lix/conductor/jedis/RedisClusterJedisProvider.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
.../java/com/netflix/conductor/service/Lifecycle.java 30.00% <0.00%> (-30.00%) 2.00% <0.00%> (-1.00%)
.../com/netflix/conductor/grpc/server/GRPCServer.java 78.57% <0.00%> (-14.29%) 4.00% <0.00%> (ø%)
...flix/conductor/core/execution/ParametersUtils.java 86.98% <0.00%> (-1.67%) 32.00% <0.00%> (+1.00%) ⬇️
...lix/conductor/dao/dynomite/queue/DynoQueueDAO.java 64.94% <0.00%> (-1.34%) 24.00% <0.00%> (+3.00%) ⬇️
...ava/com/netflix/conductor/common/run/Workflow.java 69.69% <0.00%> (-1.01%) 58.00% <0.00%> (+2.00%) ⬇️
...in/java/com/netflix/conductor/jedis/JedisMock.java 9.97% <0.00%> (-0.73%) 22.00% <0.00%> (-1.00%)
.../netflix/conductor/common/metadata/tasks/Task.java 81.15% <0.00%> (-0.03%) 82.00% <0.00%> (+2.00%) ⬇️
.../main/java/com/netflix/conductor/dao/QueueDAO.java 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
...ava/com/netflix/conductor/server/JerseyModule.java 86.36% <0.00%> (ø) 5.00% <0.00%> (-1.00%)
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 612cfef...b3cacac. Read the comment docs.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 2843

  • 0 of 4 (0.0%) changed or added relevant lines in 1 file are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.03%) to 69.539%

Changes Missing Coverage Covered Lines Changed/Added Lines %
redis-persistence/src/main/java/com/netflix/conductor/jedis/RedisClusterJedisProvider.java 0 4 0.0%
Files with Coverage Reduction New Missed Lines %
redis-persistence/src/main/java/com/netflix/conductor/jedis/JedisMock.java 2 11.44%
Totals Coverage Status
Change from base Build 2837: -0.03%
Covered Lines: 9634
Relevant Lines: 13854

💛 - Coveralls

poolConfig.setMinIdle(5);
poolConfig.setMaxTotal(1000);
return new JedisCluster(new HostAndPort(host.getHostName(), host.getPort()), poolConfig);
if (password.length() == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using StringUtils.isNotBlank to null check.

@kishorebanala
Copy link
Contributor

kishorebanala commented Apr 21, 2020

Hey @bharadwajrembar, are you still planning to work on this feature?

@bharadwajrembar
Copy link
Author

@kishorebanala I am held up at the moment. I would not be able to take on this as well as #1209 as of now.

If I find bandwidth, I will surely pick it up and inform you of the same.

@apanicker-nflx
Copy link
Collaborator

Closing due to inactivity. Please reopen if you plan to resume working on this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants