fix: Go code quality improvements #301
Merged
+157
−110
Conversation
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
- Replace io/ioutil with io (deprecated since Go 1.16) - Remove math/rand.Seed (auto-seeded since Go 1.20) - Use crypto/rand for secure random string generation - Migrate from streadway/amqp to rabbitmq/amqp091-go (official fork) - Replace k8s.io/utils/pointer with k8s.io/utils/ptr Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Extract file processing to helper functions so defer executes properly in file-service and worker packages - Store and close AMQP connection on server shutdown in control-service The defer statement inside a loop doesn't close resources until the function returns, potentially exhausting file descriptors when processing many files. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use c.Request().Context() in HTTP handlers to properly propagate cancellation signals. This ensures that when a client disconnects, the server stops processing the request instead of continuing to make etcd calls unnecessarily. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace manual mutex + map pattern with sync.Map for storing worker reply channels. sync.Map is optimized for the concurrent access pattern used here: many reads with relatively few writes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fix typo in exported struct name (missing 'u' in Execution). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- "could n ot" -> "could not" - "recursivelyt" -> "recursively" Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add defensive check to avoid panic if reply channel is not found in sync.Map (shouldn't happen in normal flow, but prevents crash). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add 2 minute timeout to wait-on health check - Add debug step that runs on failure to collect: - Pod status - Control service logs - Pod descriptions - RabbitMQ and etcd logs - Service status - Direct health endpoint check from within cluster Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Decrease timeout from 10 minutes to 3 minutes for waiting on worker pods to be ready in CI workflow.
- Use deploy/control instead of label selectors - Add --previous flag to get logs from crashed container - Fix label selector to io.kompose.service Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The new library expects heartbeat as integer seconds (heartbeat=5), not duration string (heartbeat=5s). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
mxschmitt
force-pushed
the
fix/go-code-improvements
branch
from
2684c2b to
9b94909
Compare
Replace verbose chain of != comparisons with slices.Contains for cleaner, more maintainable code. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- workertypes.IsValid(): replace manual loop with slices.Contains - worker-java transformOutput(): simplify forbidden line filtering Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
mxschmitt
force-pushed
the
fix/go-code-improvements
branch
from
d59898b to
0da0d76
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
This PR addresses several Go code quality issues identified during a code review:
Deprecated APIs & Unmaintained Packages
io/ioutilwithio(deprecated since Go 1.16)math/rand.Seed(auto-seeded since Go 1.20)crypto/randfor secure random string generationstreadway/amqptorabbitmq/amqp091-go(official fork)k8s.io/utils/pointerwithk8s.io/utils/ptrheartbeat=5notheartbeat=5s)Resource Leaks
deferexecutes properly (defer in loop doesn't close until function returns)Context Propagation
c.Request().Context()instead ofcontext.Background()in HTTP handlersConcurrency Improvements
sync.Mapfor worker reply channelsSubscribe()methodCode Quality & Modernization
slices.Containsfor membership checks (file-service MIME types, workertypes.IsValid, worker-java forbidden lines)WorkerExectionOptions→WorkerExecutionOptionsCI Improvements
Test plan
go build ./...🤖 Generated with Claude Code