[errors] return errors instead of exiting process #79
Conversation
Codecov Report
@@ Coverage Diff @@
## master #79 +/- ##
==========================================
- Coverage 45.34% 44.67% -0.67%
==========================================
Files 47 48 +1
Lines 4241 4407 +166
==========================================
+ Hits 1923 1969 +46
- Misses 2130 2211 +81
- Partials 188 227 +39
Continue to review full report at Codecov.
|
tmc
force-pushed
the
errors
branch
2 times, most recently
from
0f2c062
to
92c870a
Compare
src/cmd/cmd_query.go
Outdated
| @@ -231,9 +235,13 @@ func runQueryCmdLine(flags *sybil.FlagDefs) error { | |||
| for _, v := range allGroups { | |||
| switch t.GetColumnType(v) { | |||
| case sybil.STR_VAL: | |||
| loadSpec.Str(v) | |||
| if err := loadSpec.Str(v); err != nil { | |||
There was a problem hiding this comment.
i don't get the two statements on one line in an if
There was a problem hiding this comment.
it's a way to shorten LOC slightly but comes at a slight readability cost.. I can declare the err outside of the switch and deal with it afterwords.
tmc
force-pushed
the
errors
branch
3 times, most recently
from
713397b
to
c3b01eb
Compare
tmc
changed the title
tmc
force-pushed
the
errors
branch
6 times, most recently
from
800b607
to
baa9274
Compare
tmc
changed the title
tmc
force-pushed
the
errors
branch
3 times, most recently
from
df08e65
to
baa9274
Compare
… exit the process
There was a problem hiding this comment.
from offline discussion: mostly looks good (if you don't mind multiplying every call site by 3x), but ship it on your grpc branch, i think. i'm tired of looking at this diff (so much codes), i think you should keep exercising the code paths before we push in master
| val, err := strconv.ParseInt(iv, 10, 64) | ||
| if err == nil { | ||
| r.AddIntField(keyName, int64(val)) | ||
| val, cerr := strconv.ParseInt(iv, 10, 64) |
There was a problem hiding this comment.
why is it called cerr? to help with nesting errors?
There was a problem hiding this comment.
if I shadow "err" then I can't populate err from line 33
|
ok changed integration target to newly cut grpc branch |
* [errors] begin threading errors through instead of having sybil.Error exit the process * [errors] use errors package more extensively * [errors] add error checking to cmds * [errors] check for errors in more places * [errors] simplify returning err * [errors] add error handling to ingest, trim * [errors] add more errors.Wrap usage * [errors] add error handling to trim, inspect * [errors] add more checking to aggregate, ingest * [errors] add error checking to column unpacking * [errors] add more error coverage * [errors] relax error handling in ingestion * [errors] address a few todos * [tests] exercise BuildFilters in test
This opts to return error values instead of allowing sybil.Error to shut the process down via log.Fatalln.
I think the most likely way this change will introduce a regression is that it will not allow any errors in a set of records to go ignored. I would say it carries moderate risk.
Fixes #74