Skip to content

[CALCITE-5875] Remove unnecessary NULL checks in Redis adapter#3339

Merged
NobiGo merged 1 commit into
apache:mainfrom
taoran92:CALCITE-5875
Aug 3, 2023
Merged

[CALCITE-5875] Remove unnecessary NULL checks in Redis adapter#3339
NobiGo merged 1 commit into
apache:mainfrom
taoran92:CALCITE-5875

Conversation

@taoran92
Copy link
Copy Markdown
Member

@taoran92 taoran92 commented Jul 27, 2023

What is the purpose of the change

code cleanup, remove some unnecessary null checks for redis adapter, because the static variable must be initialized successfully unless there is an exception.

Brief change log

Remove some unnecessary null checks in redis adapter.

Verifying this change

It's minor fix can be covered by current total cases.


@AfterAll
public static void stopRedisContainer() {
if (REDIS_CONTAINER != null && REDIS_CONTAINER.isRunning()) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM


@AfterEach
public void shutDown() {
if (null != pool) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think so. When we init the pool throw exception, if, remove the condition, the code will throw NPE.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@NobiGo Thanks for reviewing, sorry for my mistake, @AfterEach will still execute if @beforeeach fails. I have fixed it.

@NobiGo NobiGo self-assigned this Aug 1, 2023
@taoran92 taoran92 force-pushed the CALCITE-5875 branch 2 times, most recently from 65e8801 to 5daae46 Compare August 1, 2023 12:06
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Aug 1, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@NobiGo NobiGo added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Aug 2, 2023
@taoran92 taoran92 changed the title [CALCITE-5875] Remove some unnecessary null checks for redis adapter [CALCITE-5875] Remove unnecessary NULL checks in Redis adapter Aug 2, 2023
@NobiGo NobiGo merged commit 5b66bd3 into apache:main Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LGTM-will-merge-soon Overall PR looks OK. Only minor things left.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants