Skip to content

HDDS-14117. Add nonStreamRead and fileRead cases to tests.#9476

Merged
sodonnel merged 2 commits intoapache:masterfrom
szetszwo:HDDS-14117
Dec 22, 2025
Merged

HDDS-14117. Add nonStreamRead and fileRead cases to tests.#9476
sodonnel merged 2 commits intoapache:masterfrom
szetszwo:HDDS-14117

Conversation

@szetszwo
Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

For comparison, we add nonStreamRead and fileRead cases to TestStreamRead.

What is the link to the Apache JIRA

HDDS-14117

How was this patch tested?

This is adding new test cases.

@rich7420
Copy link
Copy Markdown
Contributor

Thanks for the patch @szetszwo !
left some comments.

@rich7420
Copy link
Copy Markdown
Contributor

It seems a dead code.

Comment on lines 268 to 269
for (long pos = 0; pos < keySizeByte;) {
final int writeSize = Math.toIntExact(Math.min(buffer.length, keySizeByte - pos));
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.

Suggested change
for (long pos = 0; pos < keySizeByte;) {
final int writeSize = Math.toIntExact(Math.min(buffer.length, keySizeByte - pos));

Comment on lines 270 to 271
pos += writeSize;
}
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.

Suggested change
pos += writeSize;
}

@rich7420
Copy link
Copy Markdown
Contributor

overall LGTM!

@szetszwo
Copy link
Copy Markdown
Contributor Author

@rich7420 , thanks for reviewing this! Just have pushed a commit to address your comments.

@rich7420
Copy link
Copy Markdown
Contributor

thanks @szetszwo for the update!

@sodonnel sodonnel merged commit 41dd150 into apache:master Dec 22, 2025
43 checks passed
@szetszwo
Copy link
Copy Markdown
Contributor Author

@rich7420 , @sodonnel , thanks a lot for reviewing this!

@rich7420
Copy link
Copy Markdown
Contributor

@sodonnel , @szetszwo thanks!!

echonesis pushed a commit to echonesis/ozone that referenced this pull request Dec 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants