Unexport unecesserry index APIs and integrate with mulitcodec #151
Conversation
Use the codec dedicated to CAR index sorted when marshalling and unmarshalling `indexSorted`. Note, the code depends on a specific commit of `go-multicodec` `master` branch. This needs to be replaced with a tag once a release is made on the go-multicodec side later. Unexport index APIs that should not be exposed publicly. Remove `Builder` now that it is not needed anywhere. Move `insertionIndex` into `blockstore` package since that's the only place it is used. Introduce an index constructor that takes multicodec code and instantiates an index. Fix ignored errors in `indexsorted.go` during marshalling/unmarshlling. Rename index constructor functions to use consistent terminology; i.e. `new` instead if `mk`. Remove redundant TODOs in code. Relates to: - multiformats/go-multicodec#46
v2/blockstore/insertionindex.go
Outdated
| } | ||
| var ( | ||
| errUnsupported = errors.New("not supported") | ||
| insertionIndexCodec = 0x300003 |
There was a problem hiding this comment.
surely this can be replaced by go-multicodec now?
There was a problem hiding this comment.
this is the insertion index codec. Do we have a codec for that? I thought we only had one for sorted index.
There was a problem hiding this comment.
Replaced this specific constant with a wrapped multicodec.Code. Please let me know if I missed anything. thanks
There was a problem hiding this comment.
Ah of course, I had misread the name. SGTM
v2/index/index.go
Outdated
| IndexSingleSorted: mkSingleSorted, | ||
| IndexGobHashed: mkGobHashed, | ||
| IndexInsertion: mkInsertion, | ||
| // NewFromCodec constructs a new index corresponding to the given codec. |
There was a problem hiding this comment.
"given codec" is a bit too general IMO, as you only accept the CAR index codecs. How about "to the given CAR index codec"?
v2/index/index.go
Outdated
| IndexGobHashed: mkGobHashed, | ||
| IndexInsertion: mkInsertion, | ||
| // NewFromCodec constructs a new index corresponding to the given codec. | ||
| func NewFromCodec(codec multicodec.Code) (Index, error) { |
There was a problem hiding this comment.
I wonder if just New would work. The parameter is a multicodec.Code, so it's pretty evident that a codec is used.
And index.NewFromCodec(multicodec.IndexSorted) feels repetitive, whereas index.New(multicodec.IndexSorted) isn't and still works well.
* Rename constructor of index by codec to a simpler name and update docs. * Use multicodec.Code as constant instead of wrappint unit64 every time.
Use the codec dedicated to CAR index sorted when marshalling and
unmarshalling
indexSorted. Note, the code depends on a specific commitof
go-multicodecmasterbranch. This needs to be replaced with a tagonce a release is made on the go-multicodec side later.
Unexport index APIs that should not be exposed publicly. Remove
Buildernow that it is not needed anywhere. MoveinsertionIndexintoblockstorepackage since that's the only place it is used.Introduce an index constructor that takes multicodec code and
instantiates an index.
Fix ignored errors in
indexsorted.goduring marshalling/unmarshlling.Rename index constructor functions to use consistent terminology; i.e.
newinstead ifmk.Remove redundant TODOs in code.
Relates to:
go generateto generate the latest codecs multiformats/go-multicodec#46