Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2504 +/- ##
==========================================
- Coverage 79.63% 79.61% -0.03%
==========================================
Files 658 658
Lines 50420 50445 +25
Branches 736 736
==========================================
+ Hits 40154 40160 +6
- Misses 10186 10205 +19
Partials 80 80
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| if hazardous | ||
| type = 'HAZARDOUS' | ||
| cmd_type = 'HAZARDOUS' | ||
| elsif restricted | ||
| type = 'RESTRICTED' | ||
| else | ||
| type = 'NORMAL' | ||
| cmd_type = 'RESTRICTED' |
There was a problem hiding this comment.
Can cmd_type be made into an enum?
| if msg_hash['disconnect'] | ||
| @logger.info "#{@router.name}: Disconnect requested" | ||
| @tlm.disconnect(false) | ||
| next 'SUCCESS' |
There was a problem hiding this comment.
Not sure how this worked without the next 'SUCCESS' but this matches the rest of the behavior
| restricted = command.restricted | ||
| if hazardous or restricted or (self.critical_commanding == "ALL" and manual): | ||
| type = None | ||
| cmd_type = "NORMAL" |
There was a problem hiding this comment.
type is a built-in in Python so renamed
| def run(self): | ||
| for topic, msg_id, msg_hash, redis in RouterTopic.receive_telemetry(self.router, scope=self.scope): | ||
| generator = RouterTopic.receive_telemetry(self.router, scope=self.scope) | ||
| topic, msg_id, msg_hash, _redis = next(generator) |
There was a problem hiding this comment.
This being inside of a for loop meant that the old return statements would break the loop and cause the thread to exit which was detected by the operator and then restarted and immediately would reconnect. This was the source of the bug.
| RouterTopic.clear_topics(RouterTopic.topics(self.router, scope=self.scope)) | ||
| return | ||
| if msg_hash.get(b"connect"): | ||
| result = "SHUTDOWN" |
There was a problem hiding this comment.
SHUTDOWN is the only return code that breaks the loop
| if self.log_stream: | ||
| interface_or_router.stream_log_pair = StreamLogPair(interface_or_router.name, self.log_stream) | ||
| interface_or_router.start_raw_logging | ||
| interface_or_router.start_raw_logging() |
| if msg_hash[b"log_stream"].decode() == "true": | ||
| self.logger.info(f"{self.router.name}: Enable stream logging") | ||
| self.router.start_raw_logging | ||
| self.router.start_raw_logging() |
| self.logger.info(f"{self.router.name}: Disable stream logging") | ||
| self.router.stop_raw_logging | ||
| if msg_hash.get(b"router_cmd"): | ||
| self.router.stop_raw_logging() |
| pytest = "^8.3" | ||
| ruff = "^0.14" | ||
| pytest-profiling = "^1.8" | ||
| pylint = "^4.0.2" |
There was a problem hiding this comment.
What does pylint provide us that ruff doesn't already?
There was a problem hiding this comment.
well it found all the places I forgot to add () to the method call!
There was a problem hiding this comment.
When are we moving to uv ;)
There was a problem hiding this comment.
Whenever you create the PR ;)
3 participants
closes #2163