All json all the time #277
Conversation
Signed-off-by: Aaron Sutula <[email protected]>
Signed-off-by: Aaron Sutula <[email protected]>
Signed-off-by: Aaron Sutula <[email protected]>
Signed-off-by: Aaron Sutula <[email protected]>
Signed-off-by: Aaron Sutula <[email protected]>
Signed-off-by: Aaron Sutula <[email protected]>
Signed-off-by: Aaron Sutula <[email protected]>
Signed-off-by: Aaron Sutula <[email protected]>
| // ToDo: Decide how we want to apply instance id requirements | ||
| return len(instanceID) > 0 | ||
| // _, err := uuid.Parse(instanceID) | ||
| // return err == nil |
There was a problem hiding this comment.
Disabled this for now, made the issue #280
| } | ||
|
|
||
| func newCollectionFromSchema(name string, schema string, d *DB) *Collection { | ||
| func newCollection(name string, schema string, d *DB) *Collection { |
There was a problem hiding this comment.
This is where we'll want some schema verification of ID
| // if _, err := db.NewCollection(cc); err != ErrInvalidCollectionType { | ||
| // t.Fatal("the collection should be invalid") | ||
| // } | ||
| // }) |
There was a problem hiding this comment.
This is disabled until we start verifying that the schema specifies ID, tracking here #281
| @@ -114,7 +112,7 @@ func NewDBFromAddr(ctx context.Context, network net.Net, addr ma.Multiaddr, key | |||
|
|
|||
| // newDB is used directly by a db manager to create new dbs | |||
| // with the same config. | |||
| func newDB(n net.Net, id thread.ID, config *Config, collections ...CollectionConfig) (*DB, error) { | |||
| func newDB(n net.Net, id thread.ID, config *Config) (*DB, error) { | |||
There was a problem hiding this comment.
CollectionConfigs are now passed in as part of Config
| Debug: base.Debug, | ||
| EventCodec: base.EventCodec, | ||
| Debug: base.Debug, | ||
| Collections: append(base.Collections, collections...), |
There was a problem hiding this comment.
We combine the default collections that the manager was created with (using Options) with any default collections specified in the call to manager.NewDB or manager.NewDBFromAddress.
| EventCodec core.EventCodec | ||
| Debug bool | ||
| LowMem bool | ||
| Collections []CollectionConfig |
There was a problem hiding this comment.
Default collections are now part of the db config.
| sc.Collections = collections | ||
| return nil | ||
| } | ||
| } |
There was a problem hiding this comment.
Option func for setting collections
| @@ -0,0 +1,166 @@ | |||
| package db | |||
There was a problem hiding this comment.
These were the old non-json mode query tests. I figured they might cover some use cases not covered by the json mode tests, so I left them here and ported them to json mode.
There was a problem hiding this comment.
Probably some overlap, but cool
| Schema: util.SchemaFromInstance(&myCounter{}), | ||
| } | ||
|
|
||
| d, err := db.NewDBFromAddr(context.Background(), n, writerAddr, key, db.WithRepoPath(repo), db.WithCollections(cc)) |
There was a problem hiding this comment.
Heres an example of creating a db from invite and passing in a collection config so the auto start works nicely
| if err := json.Unmarshal([]byte(s), i); err != nil { | ||
| panic(err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Some utilities for converting between go types and json strings
| panic(err) | ||
| } | ||
| return string(JSON) | ||
| } |
There was a problem hiding this comment.
Utility to create a json schema from a go type
There was a problem hiding this comment.
LGTM! 👍
I'm happy to see that all those ifs distinguishing go-type/JSONMode thing are going away, and most prob some extra reflection things will be stripped off when interface{} params will go away in the future.
I wonder if going all-json might enable using better json-libraries for things in general. I'm not thinking about any particular thing, but making the point. Also, would be good to have @carsonfarmer opinion on how this may enable better/easier/etc things for indexes, considering not having to care about go-type mode.
| Debug: base.Debug, | ||
| EventCodec: base.EventCodec, | ||
| Debug: base.Debug, | ||
| Collections: append(base.Collections, collections...), |
| Schema: util.SchemaFromInstance(&myCounter{}), | ||
| } | ||
|
|
||
| d, err := db.NewDBFromAddr(context.Background(), n, writerAddr, key, db.WithRepoPath(repo), db.WithCollections(cc)) |
| @@ -9,6 +9,7 @@ import ( | |||
| ma "github.com/multiformats/go-multiaddr" | |||
There was a problem hiding this comment.
A quick refresher: This e2e_counter was some of the first e2e things that I created when we had a first working version of DB. We have chatted some times to remove this code if may be confusing or adding too much burden to refactors or maintainability. No strong opinion, but just refreshing this since I originally made this example... no feelings with it. If it adds value in some direction, np with leaving this too.
The books example was made roughly at the same time... but feels that example it's much nicer to show somebody a minimal way of using things.
Just leaving al this as a note here.
There was a problem hiding this comment.
Ah ok. If anyone has some strong feeling about removing it, I'm fine with that, but also don't have a strong opinion.
|
Thanks for the review @jsign. Yea, all that reflect stuff goes away with the removal of |
| sc.Collections = collections | ||
| return nil | ||
| } | ||
| } |
| Schema: util.SchemaFromInstance(&myCounter{}), | ||
| } | ||
|
|
||
| d, err := db.NewDBFromAddr(context.Background(), n, writerAddr, key, db.WithRepoPath(repo), db.WithCollections(cc)) |
This PR removes the two db modes of normal go mode and json mode. The only mode now is what was called json mode, there is no "mode" nomenclature any more.
Also snuck in an update to better support creating
Collections as part of db creation.The
Collectionandjsonpatcherapis still takeinterface{}arguments event though all the data isstringand*string. I plan on cleaning that up in the next PR where all methods will explicitly accept and return[]bytefor json data.