Address review comments and add unit tests for internal/query - #3473
Merged
airlock-confluentinc[bot] merged 1 commit intoAug 24, 2026
Conversation
- Backtick-format flag names in error/suggestion strings for consistency with the rest of the repo's output conventions. - Fix the Hidden-gating comment's precedent: unified-stream-manager doesn't exist in this repo; point at the private link ingress endpoint command instead, which is the actual precedent for this pattern. - Correct the README's stale "no token refresh" limitation now that Options.RefreshToken exists, and note that the default 10-minute timeout is on the same order as the dataplane token's lifetime so it rarely matters in practice. - Add internal/query/command_test.go covering buildQueryProperties, printQueryResult, refreshGatewayToken, stopStatement and handleQueryError, raising this package's coverage from 0% to address the SonarQube new-code coverage gate. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
🎉 All Contributor License Agreements have been signed. Ready to merge. |
airlock-confluentinc
Bot
merged commit Aug 24, 2026
24468a1
into
flink-sync-snapshot-query
2 of 3 checks passed
airlock-confluentinc
Bot
deleted the
flink-sync-snapshot-query-review-fixes
branch
August 24, 2026 18:19
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses outstanding review feedback on #3466 and the failing SonarQube quality gate (new-code coverage 39.4% < required 80%).
Review comments addressed
--max-rows,--raw,-o json/-o yaml,--timeout) that were using double quotes, per this repo's output-formatting convention.unified-stream-manageras precedent for the Hidden-until-targeted feature-flag gating pattern — that package doesn't exist in this repo. Pointed at the actual precedent instead (internal/network/command_access_point_private_link_ingress_endpoint.go).Options.RefreshTokenalready exists in this PR. Reworded to note the refresh is best-effort/not retry-aware, and that in practice it rarely matters since the default 10-minute--timeoutis on the same order as the dataplane token's own lifetime.CI fix
internal/query/command.goshipped with zero unit tests, which was the main driver of the failing coverage gate. Addedinternal/query/command_test.gocovering:buildQueryPropertiesprintQueryResult(table and serialized/raw output, empty results, operation column)refreshGatewayToken(valid token, expired token, refresh failure)stopStatement(success and failure)handleQueryError(all branches: unbounded, canceled, deadline exceeded, 404 vs other results-fetch errors, generic fallback)New(command construction/flag registration)Coverage for the package goes from 0% to 51.6%.
go vet,golangci-lint run, andmake lint-cliare clean.🤖 Generated with Claude Code