Skip to content

JedisClusterCRC16.getSlot() wraps NullPointerException #1774

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Aug 13, 2018
Merged

JedisClusterCRC16.getSlot() wraps NullPointerException #1774

merged 3 commits into from
Aug 13, 2018

Conversation

@sazzad16
Copy link
Contributor

@sazzad16 sazzad16 commented Feb 13, 2018

  • This is similar to SafeEncoder.encode(String).
  • This helps JedisClusterCommand specially in case of multi key operations.

Also includes,

  • Move CRC16 tests in one place.
  • Fix a wrong usage of DEFAULT_REDIRECTIONS.

@sazzad16
Move CRC16 tests in one place.
04730b4 
Also, fixed a wrong usage of DEFAULT_REDIRECTIONS.
@sazzad16
JedisClusterCRC16.getSlot() wraps NullPointerException
6d63bdc 
- This is similar to SafeEncoder.encode(String).
- This helps JedisClusterCommand specially in case of multi key operations.
@sazzad16 sazzad16 added this to the 3.0.0 milestone Feb 13, 2018
@sazzad16 sazzad16 mentioned this pull request Feb 13, 2018
Closed
@sazzad16
Copy link
Contributor Author

sazzad16 commented Feb 13, 2018

@marcosnils @HeartSaVioR

@marcosnils
Copy link
Contributor

marcosnils commented Aug 12, 2018 •
edited
Loading

LGTM. Can be merged after rebased. Shouldn't we schedule it for 2.9.1?. Seems like a bugfix to me.

@sazzad16
Copy link
Contributor Author

sazzad16 commented Aug 13, 2018

@marcosnils 3.0.0 is the one to go for. Because

  1. Previously, a null key would cause NullPointerException. What if people have implementation catching this one?

  2. JedisClusterOperationException is only available in 3.0.0.

@marcosnils
Copy link
Contributor

marcosnils commented Aug 13, 2018

Previously, a null key would cause NullPointerException. What if people have implementation catching this one?

This is a bug and shouldn't be thrown.

JedisClusterOperationException is only available in 3.0.0.

We can't do anything about this. Let's schedule it for 3.0 then :)

@sazzad16 sazzad16 merged commit cf31202 into redis:master Aug 13, 2018
@sazzad16
Copy link
Contributor Author

sazzad16 commented Aug 13, 2018

Merged into master.

@sazzad16 sazzad16 deleted the cluster-command-null-empty-2 branch August 13, 2018 06:00
joyang1 pushed a commit to joyang1/jedis that referenced this pull request Dec 27, 2018
@sazzad16
JedisClusterCRC16.getSlot() wraps NullPointerException (redis#1774)
d1af6b4 
* This is similar to SafeEncoder.encode(String).
* This helps JedisClusterCommand specially in case of multi key operations.

Also includes,
* Move CRC16 tests in one place.
* Fix a wrong usage of DEFAULT_REDIRECTIONS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

3.0.0

Development

Successfully merging this pull request may close these issues.

2 participants

@sazzad16 @marcosnils