Skip to content

Commit

Permalink
fix(dry-run): fix logbook conflict with dry-run
Browse files Browse the repository at this point in the history
dry run was broken by logbook, which constructs a backing store on the fly, and passes it to logbook, which was getting upset.
  • Loading branch information
b5 committed Feb 21, 2020
1 parent 67b222f commit 4c051b6
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 14 deletions.
4 changes: 2 additions & 2 deletions base/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ func init() {
func newTestRepo(t *testing.T) repo.Repo {
mapStore := cafs.NewMapstore()
mux := qfs.NewMux(map[string]qfs.Filesystem{
"map": mapStore,
"cafs": mapStore,
"local": mapStore,
"cafs": mapStore,
})
mr, err := repo.NewMemRepo(testPeerProfile, mapStore, mux, profile.NewMemStore())
if err != nil {
Expand Down
37 changes: 26 additions & 11 deletions base/save.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ func SaveDataset(ctx context.Context, r repo.Repo, str ioes.IOStreams, changes *
// dry-runs store to an in-memory repo
r, err = repo.NewMemRepo(pro, cafs.NewMapstore(), r.Filesystem(), profile.NewMemStore())
if err != nil {
log.Debugf("creating new memRepo: %s", err)
return
}
}
Expand Down Expand Up @@ -163,14 +164,17 @@ func CreateDataset(ctx context.Context, r repo.Repo, streams ioes.IOStreams, ds,

pro, err = r.Profile()
if err != nil {
log.Debugf("getting repo profile: %s", err)
return
}

if err = ValidateDataset(ds); err != nil {
log.Debugf("ValidateDataset: %s", err)
return
}

if path, err = dsfs.CreateDataset(ctx, r.Store(), ds, dsPrev, r.PrivateKey(), pin, force, shouldRender); err != nil {
log.Debugf("dsfs.CreateDataset: %s", err)
return
}
if ds.PreviousPath != "" && ds.PreviousPath != "/" {
Expand All @@ -192,29 +196,40 @@ func CreateDataset(ctx context.Context, r repo.Repo, streams ioes.IOStreams, ds,
Path: path,
}

// TODO (b5) - when we're doing a dry run, this is putting a reference into
// a blank in-memory repo, and is needed by the ReadDataset call below. I'd
// prefer this move into the `if !dryRun` clause below, or be dropped entirely
// in favour of dscache
if err = r.PutRef(ref); err != nil {
log.Debugf("r.PutRef: %s", err)
return
}

// TODO (b5): confirm these assignments happen in dsfs.CreateDataset with tests
ds.ProfileID = pro.ID.String()
ds.Peername = pro.Peername
ds.Path = path
action, err := r.Logbook().WriteVersionSave(ctx, ds)
if err != nil && err != logbook.ErrNoLogbook {
return
}
dscache := r.Dscache()
if dscache != nil && !dscache.IsEmpty() {
log.Info("dscache: update and save new version")
err = dscache.Update(action)
if err != nil {
log.Error(err)

if !dryRun {
action, err := r.Logbook().WriteVersionSave(ctx, ds)
if err != nil && err != logbook.ErrNoLogbook {
return ref, err
}

dscache := r.Dscache()
if dscache != nil && !dscache.IsEmpty() {
log.Info("dscache: update and save new version")
err = dscache.Update(action)
if err != nil {
log.Error(err)
}
}

}

if err = ReadDataset(ctx, r, &ref); err != nil {
return
log.Debugf("ReadDataset: %s", err)
return ref, err
}

// need to open here b/c we might be doing a dry-run, which would mean we have
Expand Down
2 changes: 1 addition & 1 deletion repo/mem_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type MemRepo struct {
// TODO (b5) - need a better mem-repo constructor, we don't need a logbook for
// all test cases
func NewMemRepo(p *profile.Profile, store cafs.Filestore, fsys qfs.Filesystem, ps profile.Store) (*MemRepo, error) {
book, err := logbook.NewJournal(p.PrivKey, p.Peername, fsys, "/map/logbook")
book, err := logbook.NewJournal(p.PrivKey, p.Peername, fsys, "/mem/logbook")
if err != nil {
return nil, err
}
Expand Down

0 comments on commit 4c051b6

Please sign in to comment.