Skip to content

SYM-2983: Resume transfer of large batches using HTTP Range headers - #973

Draft
evan-miller-jumpmind wants to merge 11 commits into
release/3.18from
feature/2983_resume_transfer_of_large_batches
Draft

SYM-2983: Resume transfer of large batches using HTTP Range headers#973
evan-miller-jumpmind wants to merge 11 commits into
release/3.18from
feature/2983_resume_transfer_of_large_batches

Conversation

@evan-miller-jumpmind

Copy link
Copy Markdown
Contributor

No description provided.

@evan-miller-jumpmind evan-miller-jumpmind self-assigned this Aug 4, 2026
@evan-miller-jumpmind
evan-miller-jumpmind requested a review from a team as a code owner August 4, 2026 18:28
@evan-miller-jumpmind

Copy link
Copy Markdown
Contributor Author

I'm taking a look at the Sonar alerts.

@w3bbd3v-jm Andrew Webb (w3bbd3v-jm) left a comment

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.

Are we sure we want to include this in 3.18? The scope for 3.18 is already rather large and we still have a lot of testing to go. Including another new feature seems to bloat the release even more than it is. Maybe it's time we open release/3.19?

@evan-miller-jumpmind

Copy link
Copy Markdown
Contributor Author

Are we sure we want to include this in 3.18? The scope for 3.18 is already rather large and we still have a lot of testing to go. Including another new feature seems to bloat the release even more than it is. Maybe it's time we open release/3.19?

We're leaning towards including it in 3.18, but we still need to make a final decision. I created a lab environment in symmetric-labs to make it easier to test this change.

@evan-miller-jumpmind

Copy link
Copy Markdown
Contributor Author

I fixed most of the Sonar alerts and now only 8 remain.

The Sonar warning in FileSyncService.handleExtractionError() is inaccurate. The currentBatch variable can't use log.error()'s built-in formatting or else e will be treated as a variable to be substituted into the string rather than an exception, which will cause an exception to be thrown.

The 3 Sonar failures related to closing streams are false positives. The HttpOutgoingTransport and HttpIncomingTransport own these streams and handle closing them.

The Sonar failure on FileSyncService.processUnzippedBatches() is on pre-existing logic that I extracted into a renamed method. It would be high-risk to resolve it, so I left it as it is.

Three of the alerts in StagedResource are on code that I didn't touch at all. The root cause of the failure in getWriter() is actually in buildFile(), which is untouched.

@pavel-jm

Copy link
Copy Markdown
Contributor

new feature seems to bloat

We have reviewed this feature in the team meeting.
Let me know if you'd like us to schedule another meeting on this topic?

@pavel-jm Pavel_JM (pavel-jm) left a comment

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.

Thanks! I'll share additional details over chat

@w3bbd3v-jm

Copy link
Copy Markdown
Contributor

new feature seems to bloat

We have reviewed this feature in the team meeting. Let me know if you'd like us to schedule another meeting on this topic?

My vote is for 3.19.

@evan-miller-jumpmind

Copy link
Copy Markdown
Contributor Author

I'm taking a look at the 7 new Sonar alerts.

@pavel-jm

Copy link
Copy Markdown
Contributor

It appears there are two SonarQube findings worth a follow up.
Both are for the same resource-leak prevention practice:

Use try-with-resources or close this "OutputStream" in a "finally" clause.

@evan-miller-jumpmind

Copy link
Copy Markdown
Contributor Author

It appears there are two SonarQube findings worth a follow up. Both are for the same resource-leak prevention practice:

Use try-with-resources or close this "OutputStream" in a "finally" clause.

When I looked at this before, I found that the 3 Sonar failures related to closing streams are false positives. The HttpOutgoingTransport and HttpIncomingTransport own these streams and handle closing them.

@evan-miller-jumpmind

Copy link
Copy Markdown
Contributor Author

We decided to move this feature to 3.19. I'll mark this PR as a draft for now and change the target branch to release/3.19 once it's available.

@evan-miller-jumpmind
evan-miller-jumpmind marked this pull request as draft August 20, 2026 13:57
@sonarqubecloud

sonarqubecloud Bot commented Sep 1, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
E Security Rating on New Code (required ≥ A)
E Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

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