Fix sonarqube quality issues - #3836
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3836 +/- ##
==========================================
- Coverage 79.30% 79.28% -0.02%
==========================================
Files 896 896
Lines 67371 67373 +2
Branches 2608 2608
==========================================
- Hits 53426 53420 -6
- Misses 13276 13287 +11
+ Partials 669 666 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
The updated asyncio shutdown flow can have cleanup interrupted by cancellation and also risks silently suppressing unexpected task exceptions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Implements small “quality” refactors flagged by SonarQube across Python and Ruby code, primarily around comparison helpers, environment-variable access, and asyncio cancellation/cleanup behavior.
Changes:
- Add
@total_orderingtoSegmentedPolynomialConversion.Segmentto synthesize missing comparison operators. - Reduce repeated env-var lookups in
JsonApi#_generate_urlby cachingOPENC3_OPERATOR_HOSTNAME. - Refactor asyncio task cancellation/cleanup logic in the throughput server example.
File summaries
| File | Description |
|---|---|
| openc3/python/openc3/conversions/segmented_polynomial_conversion.py | Adds total_ordering to complete ordering operations on Segment. |
| openc3/lib/openc3/io/json_api.rb | Caches OPENC3_OPERATOR_HOSTNAME lookup to avoid repeated ENV access. |
| examples/throughput_server/throughput_server.py | Refactors cancellation/cleanup logic for background tasks and server shutdown. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
| # A polynomial conversion segment which applies the conversion from the | ||
| # lower bound (inclusive) until another segment's lower bound is | ||
| # encountered. | ||
| @total_ordering |
There was a problem hiding this comment.
Neat decorator, it will create the remaining rich comparison functions as well, is that alright?
There was a problem hiding this comment.
Yes, we're only using the less than and equal for the logic. Note that our less than only compares bounds while the equal compares bounds and coeffs. Not sure if the comparison with coeffs is strictly necessary in production but it looks like it helps our unit tests.
What changed
Implement a few SonarQube quality fixes. No functionality change.
Why it changed
SonarQube marked it as high
Testing strategy
CI/CD