Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1581 +/- ##
=======================================
Coverage 95.93% 95.93%
=======================================
Files 44 44
Lines 3270 3271 +1
=======================================
+ Hits 3137 3138 +1
Misses 133 133 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
DominicOram
left a comment
DominicOram
left a comment
There was a problem hiding this comment.
Awesome, thank you for the quick fix! The message that is logged is good, just disagree about the level. Will approve assuming that you increase the level.
| for err in e.errors() | ||
| ] | ||
|
|
||
| LOGGER.info("Error submitting task: %s - %s", task_request, e) |
There was a problem hiding this comment.
Should: info seems low. I would say at least warn, maybe even error.
There was a problem hiding this comment.
It's not really an error from blueapi's perspective if the user provided invalid input. Happy to see what others think though
There was a problem hiding this comment.
It is a client error but a server info so I agree with Peter that it should only be logged as info on server side.
This is similar to how fastapi handles a request which has correct syntax but wrong instruction, and we are sending the correct response for it 422 same as fastapi
There was a problem hiding this comment.
Sure, I think it will make support slightly easier if it's error but I'm not that fussed as long as the information is there
There was a problem hiding this comment.
merge it for now, but if it proves to be frustrating on the user side we can change it
No description provided.