Issue 495 - misc cleanup #1001
Issue 495 - misc cleanup #1001
Conversation
chu11
force-pushed
the
issue495-jansson-cleanup
branch
from
99b1ea7
to
0aaa0ef
Compare
|
In hindsight, 57a29a9 probably shouldn't have been split off into this "cleanup" set of patches. Re-pushed without it, I'll add that to the beefier PR later on. |
|
Just took a spin through these and looks like good cleanup to me! |
Codecov Report
@@ Coverage Diff @@
## master #1001 +/- ##
==========================================
+ Coverage 76.17% 76.18% +<.01%
==========================================
Files 153 153
Lines 26545 26556 +11
==========================================
+ Hits 20220 20231 +11
Misses 6325 6325
Continue to review full report at Codecov.
|
|
looks like most of the code coverage misses are in error handling. Lines in heaptrace were missed, I guess there are no tests for that or something. I'll rebase and push on new master. |
|
Sounds good, I'll merge once that passes travis |
Fix incorrect branch logic. In prior code, if seq_cmp_and_set() succeeds, the first half of the "||" branch logic fails, leading to a subsequent call to seq_set(). Instead, we wish to call seq_cmp_and_set() or seq_set() only once, based on logic conditions.
Rename internal block json_str declaration, it collides with function block's additional json_str declaration.
For resource-hwloc.reload response, do not send unnecessary "empty json array" payload. Instead do not send a payload.
Pass format string to flux_respondf() consistent to how it is done in rest of Flux.
Properly capture errnum in calls to flux_request_decode() so it can be sent in response.
In the event no errors occur in cmb.exec.write callback, errnum was never set to 0, so a error was always returned to the caller.
Add error checking/logging in resource-hwloc.reload and resource-hwloc.topo callbacks.
Create common "goto" for sending an error response in resource-hwloc.topo callback. Add log for failed RPC too.
chu11
force-pushed
the
issue495-jansson-cleanup
branch
from
0aaa0ef
to
84cc37b
Compare
|
I know 57a29a9 is gone from here now, but one thing to be careful of with boolean in Lua is that Let me peek at the flux-cron script and see if we can get it to properly return a boolean for the --kill option. |
|
@grondo Huh, not what I would have guessed :-) |
Just an update for @chu11, I think your |
|
@grondo Ahh, cool. I'll add a comment to the patch so that's clear. Thanks! |
While doing a lot of conversion to jansson-style functions in #495, fixed of variety of cleanup things along the way. Decided to split them all off onto another pull request, before sending over the beefier changes.
Most fixes are obvious, the notable exception may be 57a29a9. It was necessary b/c jansson is pickier on types. (i.e.
"1"is a string while1is an int). My lua is weak, would appreciate look to make suretobooleanlocal function is a smart solution?