Skip to content

Commit

Permalink
fix(dry-run): dry-runs must never add to the refstore
Browse files Browse the repository at this point in the history
fixed this by unwrapping an extraneous call to load a newly saved dataset, shedding a
canonicalization call.

also added a number of guards within lib.DatasetRequests.Save to prevent dry-run from
messing everything up.
  • Loading branch information
b5 committed Apr 6, 2020
1 parent 734a800 commit d1b71c2
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 19 deletions.
Binary file modified api/testdata/api.snapshot
Binary file not shown.
31 changes: 15 additions & 16 deletions base/save.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,32 +199,31 @@ 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

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

ds.ProfileID = pro.ID.String()
ds.Peername = pro.Peername
ds.Path = path

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

if err = ReadDataset(ctx, r, &ref); err != nil {
log.Debugf("ReadDataset: %s", err)
ds, err = dsfs.LoadDataset(ctx, r.Store(), ref.Path)
if err != nil {
return ref, err
}
ds.ProfileID = pro.ID.String()
ds.Name = ref.Name
ds.Peername = ref.Peername
ds.Path = path
ref.Dataset = ds

// need to open here b/c we might be doing a dry-run, which would mean we have
// references to files in a store that won't exist after this function call
Expand Down
9 changes: 6 additions & 3 deletions lib/datasets.go
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ func (r *DatasetRequests) Save(p *SaveParams, res *reporef.DatasetRef) (err erro
}

// If the dscache doesn't exist yet, it will only be created if the appropriate flag enables it.
if p.UseDscache {
if p.UseDscache && !p.DryRun {
c := r.node.Repo.Dscache()
c.CreateNewEnabled = true
}
Expand All @@ -598,7 +598,7 @@ func (r *DatasetRequests) Save(p *SaveParams, res *reporef.DatasetRef) (err erro
}

// TODO (b5) - this should be integrated into base.SaveDataset
if fsiPath != "" {
if fsiPath != "" && !p.DryRun {
datasetRef.FSIPath = fsiPath
if err = r.node.Repo.PutRef(datasetRef); err != nil {
return err
Expand All @@ -612,6 +612,9 @@ func (r *DatasetRequests) Save(p *SaveParams, res *reporef.DatasetRef) (err erro
}

if p.Publish {
if p.DryRun {
return fmt.Errorf("can't use publish & dry-run together")
}
var publishedRef reporef.DatasetRef
err = r.SetPublishStatus(&SetPublishStatusParams{
Ref: datasetRef.String(),
Expand All @@ -627,7 +630,7 @@ func (r *DatasetRequests) Save(p *SaveParams, res *reporef.DatasetRef) (err erro

*res = datasetRef

if fsiPath != "" {
if fsiPath != "" && !p.DryRun {
// Need to pass filesystem here so that we can read the README component and write it
// properly back to disk.
fsi.WriteComponents(res.Dataset, datasetRef.FSIPath, r.inst.node.Repo.Filesystem())
Expand Down

0 comments on commit d1b71c2

Please sign in to comment.