perf!: Changed tuplelist deserialization. #76
Conversation
Signed-off-by: Alwin Zomotor <[email protected]>
…ger useful Signed-off-by: Alwin Zomotor <[email protected]>
…ger useful Signed-off-by: Alwin Zomotor <[email protected]>
- fake castor client always returned empty byte array; - removed unused functions Signed-off-by: Alwin Zomotor <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #76 +/- ##
==========================================
- Coverage 82.29% 82.02% -0.28%
==========================================
Files 29 29
Lines 3158 3138 -20
==========================================
- Hits 2599 2574 -25
- Misses 485 488 +3
- Partials 74 76 +2
Flags with carried forward coverage won't be shown. Click here to find out more. see 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Signed-off-by: Alwin Zomotor <[email protected]>
| if err != nil { | ||
| return nil, err | ||
| } | ||
| ts.logger.Debugw("Fetched new tuples from Castor", "RequestID", requestID) | ||
| tupleData, err := ts.tupleListToByteArray(tupleList) |
There was a problem hiding this comment.
I think we should at least check whether received data fits to the requested tuple parameters (length of array equals number of requested tuples x length of requested tuple type, or sth like that)
There was a problem hiding this comment.
must be part of the client if GetTuples returns a Reader as proposed above
There was a problem hiding this comment.
Please update Copyright header
There was a problem hiding this comment.
Please update Copyright header
There was a problem hiding this comment.
Please update Copyright header
There was a problem hiding this comment.
Please update Copyright header
| @@ -66,17 +66,16 @@ func (c *Client) GetTuples(count int32, tt TupleType, requestID uuid.UUID) (*Tup | |||
| if err != nil { | |||
| return nil, fmt.Errorf("communication with castor failed: %s", err) | |||
| } | |||
| bodyBytes, err := ioutil.ReadAll(resp.Body) | |||
There was a problem hiding this comment.
as the byte[] to is later written to a pipe (file) consumed by the MPC runtime, wouldn't it make sense to return the Body (ReadCloser) directly. This would eliminate the intermediate step of "chaching" as a byte[] first.
There was a problem hiding this comment.
possibly as an additional asStream / asReader method
There was a problem hiding this comment.
one could utilize io.Copy for implementation
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return nil, fmt.Errorf("getting tuples failed for \"%s\" with response code #%d: %s", req.URL, resp.StatusCode, string(bodyBytes)) | ||
| } | ||
| tuples := &TupleList{} | ||
| err = json.NewDecoder(resp.Body).Decode(tuples) | ||
| if err != nil { |
There was a problem hiding this comment.
that's unnecessary code now, isn't it?
| "github.com/asaskevich/govalidator" | ||
| ) | ||
|
|
||
| // AbstractClient is an interface for castor tuple client. | ||
| type AbstractClient interface { | ||
| GetTuples(tupleCount int32, tupleType TupleType, requestID uuid.UUID) (*TupleList, error) |
There was a problem hiding this comment.
Is TupleList still used at all?
No description provided.