engine: Support writing already encoded objects#2801
Conversation
cthulhu-rider
commented
Apr 5, 2024
- reverse side of engine: Support reading objects without decoding #2753
There is an object testing package itself which is usually imported as `objecttest`. Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
9a73762 to
9ddcbb3
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2801 +/- ##
==========================================
+ Coverage 22.13% 22.17% +0.03%
==========================================
Files 793 793
Lines 47586 47607 +21
==========================================
+ Hits 10533 10556 +23
+ Misses 36148 36147 -1
+ Partials 905 904 -1 ☔ View full report in Codecov by Sentry. |
roman-khimov
left a comment
There was a problem hiding this comment.
Seems to be legit and can go into 0.41.0.
| type PutPrm struct { | ||
| obj *objectSDK.Object | ||
|
|
||
| binSet bool |
There was a problem hiding this comment.
binSet prevents duality b/w uncalled and called with nil cases
There was a problem hiding this comment.
called with nil cases
is it valid?
There was a problem hiding this comment.
depends on the caller. This is his responsibility, docs say that
There was a problem hiding this comment.
undesired complexity to me. calling with empty bytes ends with no error (i guess) while that is incorrect for sure
| if err != nil { | ||
| return PutRes{}, fmt.Errorf("cannot marshal object: %w", err) | ||
| } | ||
| // TODO: currently, we don't need to calculate prm.hdrLen in this case. |
There was a problem hiding this comment.
yes, this won't be lost
There was a problem hiding this comment.
how can you be sure? we had a work once where the dev searched for such lost cases
There was a problem hiding this comment.
how can you be sure?
this is what git is made for
There was a problem hiding this comment.
git is made for
searching for TODO strings? my point is that TODO is about smth that should be done. good to have a corresponding issue then. otherwise that is not TODO
There was a problem hiding this comment.
To me it's not even a TODO, just a remark on the specifics of the current code. It's nothing actionable at this point.
| } | ||
|
|
||
| // SetObjectBinary allows to provide the already encoded object in | ||
| // [StorageEngine] format. Object header must be a prefix with specified length. |
There was a problem hiding this comment.
btw not that clear what such a format is
| type PutPrm struct { | ||
| obj *objectSDK.Object | ||
|
|
||
| binSet bool |
There was a problem hiding this comment.
undesired complexity to me. calling with empty bytes ends with no error (i guess) while that is incorrect for sure