Skip to content

Use IllegalStateException to replace invalid JedisDataException #2393

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 18 commits into from
Mar 31, 2021
Merged

Use IllegalStateException to replace invalid JedisDataException #2393

merged 18 commits into from
Mar 31, 2021

Conversation

@sazzad16
Copy link
Contributor

@sazzad16 sazzad16 commented Mar 1, 2021 •
edited
Loading

Reference #2360 (comment)

Closes #2360
Closes #1733

sazzad16 and others added 13 commits November 21, 2018 00:53
@sazzad16
Introduce JedisBatchOperationException
21125f0 
- Pipeline and Transaction related exceptions, which do not come from Redis, are changed to throw JedisBatchOperationException.
- SafeEncoder is changed to throw JedisException.

Based on discussions in redis#1183
@sazzad16
Merge branch 'master' into confusing-JedisDataException
c8bfd43
@sazzad16
Merge branch 'master' into confusing-JedisDataException
701a3dd
@sazzad16
Update JedisTest.java
feec754
@sazzad16
Update JedisTest.java
6eac07e
@sazzad16
Update JedisWithCompleteCredentialsTest.java
226d0a0
@sazzad16
Merge remote-tracking branch 'redis/master' into confusing-JedisDataE…
27aab63 
…xception
@sazzad16
Merge remote-tracking branch 'redis/master' into confusing-JedisDataE…
310999d 
…xception
@sazzad16
backward compatibility with JedisBatchOperationException
56344d6
@sazzad16
address review
a83d64d
@sazzad16
remove deprecation
0b47142
@sazzad16
Use IllegalStateException to replace invalid JedisDataException
ebad1f5
@sazzad16
remove unused import
6652ee4
@sazzad16 sazzad16 requested a review from gkorland March 1, 2021 11:03
@sazzad16 sazzad16 mentioned this pull request Mar 1, 2021
Closed
mp911de
mp911de previously approved these changes Mar 1, 2021
View reviewed changes
Copy link
Contributor

@mp911de mp911de left a comment

Choose a reason for hiding this comment

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

Looks good.

sazzad16
sazzad16 commented Mar 1, 2021
View reviewed changes
sazzad16
sazzad16 commented Mar 1, 2021
View reviewed changes
@mp911de
Copy link
Contributor

mp911de commented Mar 1, 2021

A thought on backward-compatibility: These exceptions aren't really ones where an application can recover from, especially when trying to send null. The only case where I can see it would matter if there's an interactive application such as a Java CLI for Redis built using Jedis.

@sazzad16 sazzad16 added this to the 4.0.0 milestone Mar 1, 2021
gkorland
gkorland previously approved these changes Mar 2, 2021
View reviewed changes
dengliming
dengliming previously approved these changes Mar 15, 2021
View reviewed changes
sazzad16
sazzad16 commented Mar 15, 2021
View reviewed changes
sazzad16
sazzad16 commented Mar 15, 2021
View reviewed changes
sazzad16
sazzad16 commented Mar 15, 2021
View reviewed changes
sazzad16
sazzad16 commented Mar 15, 2021
View reviewed changes
@sazzad16 sazzad16 dismissed stale reviews from dengliming and gkorland via 4df1e10 March 15, 2021 13:09
@sazzad16 sazzad16 merged commit 3a48633 into redis:master Mar 31, 2021
@sazzad16 sazzad16 deleted the batch-op-ex-2 branch March 31, 2021 14:35
@sazzad16 sazzad16 mentioned this pull request Dec 12, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@gkorland gkorland gkorland left review comments

@mp911de mp911de mp911de left review comments

@dengliming dengliming dengliming left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@sazzad16 @mp911de @gkorland @dengliming