fix(save): Remove dry-run, recall, return-body from save path

api/datasets.go

@@ -483,13 +483,10 @@ func (h *DatasetHandlers) saveHandler(w http.ResponseWriter, r *http.Request) {
  Ref: ref.AliasString(),
  Dataset: ds,
  Private: r.FormValue("private") == "true",
- DryRun: r.FormValue("dry_run") == "true",
- ReturnBody: r.FormValue("return_body") == "true",
  Force: r.FormValue("force") == "true",
  ShouldRender: !(r.FormValue("no_render") == "true"),
  NewName: r.FormValue("new") == "true",
  BodyPath: r.FormValue("bodypath"),
- Recall: r.FormValue("recall"),
  Drop: r.FormValue("drop"),
 
  ConvertFormatToPrev: true,

api/datasets_test.go

@@ -133,8 +133,7 @@ func TestDatasetHandlers(t *testing.T) {
  },
  map[string]string{
  "peername": "peer",
- "name": "cities_dry_run",
- "dry_run": "true",
+ "name": "cities_2",
  },
  },
  }

base/revisions.go

@@ -10,29 +10,6 @@ import (
  "github.com/qri-io/qri/dsref"
 )
 
-// Recall loads revisions of a dataset from history of a resolved dataset
-// reference
-func Recall(ctx context.Context, fs qfs.Filesystem, ref dsref.Ref, revStr string) (*dataset.Dataset, error) {
- if revStr == "" {
- return &dataset.Dataset{}, nil
- }
- if ref.Path == "" {
- return nil, fmt.Errorf("can only recall from a resolved reference with a path value")
- }
-
- revs, err := dsref.ParseRevs(revStr)
- if err != nil {
- return nil, err
- }
-
- res, err := LoadRevs(ctx, fs, ref, revs)
- if err != nil {
- return nil, err
- }
-
- return res, nil
-}
-
 // LoadRevs grabs a component of a dataset that exists <n>th generation ancestor
 // of the referenced version, where presence of a component in a previous snapshot constitutes ancestry
 func LoadRevs(ctx context.Context, fs qfs.Filesystem, ref dsref.Ref, revs []*dsref.Rev) (res *dataset.Dataset, err error) {

base/revisions_test.go

@@ -12,22 +12,6 @@ import (
  "github.com/qri-io/qri/dsref"
 )
 
-func TestRecall(t *testing.T) {
- ctx := context.Background()
- r := newTestRepo(t)
- ref := addNowTransformDataset(t, r)
-
- _, err := Recall(ctx, r.Filesystem(), ref, "")
- if err != nil {
- t.Error(err)
- }
-
- _, err = Recall(ctx, r.Filesystem(), ref, "tf")
- if err != nil {
- t.Error(err)
- }
-}
-
 func TestLoadRevisions(t *testing.T) {
  ctx := context.Background()
  r := newTestRepo(t)

cmd/save.go

@@ -1,7 +1,6 @@
 package cmd
 
 import (
- "encoding/json"
  "fmt"
 
  "github.com/qri-io/dataset"
@@ -65,13 +64,12 @@ commit message and title to the save.`,
  cmd.Flags().StringVarP(&o.Message, "message", "m", "", "commit message for save")
  cmd.Flags().StringVarP(&o.BodyPath, "body", "", "", "path to file or url of data to add as dataset contents")
  cmd.MarkFlagFilename("body")
- cmd.Flags().StringVarP(&o.Recall, "recall", "", "", "restore revisions from dataset history")
  // cmd.Flags().BoolVarP(&o.ShowValidation, "show-validation", "s", false, "display a list of validation errors upon adding")
  cmd.Flags().StringSliceVar(&o.Secrets, "secrets", nil, "transform secrets as comma separated key,value,key,value,... sequence")
- cmd.Flags().BoolVar(&o.DryRun, "dry-run", false, "simulate saving a dataset")
+ cmd.Flags().BoolVar(&o.DeprecatedDryRun, "dry-run", false, "deprecated: use `qri apply` instead")
  cmd.Flags().BoolVar(&o.Force, "force", false, "force a new commit, even if no changes are detected")
  cmd.Flags().BoolVarP(&o.KeepFormat, "keep-format", "k", false, "convert incoming data to stored data format")
- // TODO(dlong): --no-render is deprecated, viz are being phased out, in favor of readme.
+ // TODO(dustmop): --no-render is deprecated, viz are being phased out, in favor of readme.
  cmd.Flags().BoolVar(&o.NoRender, "no-render", false, "don't store a rendered version of the the visualization")
  cmd.Flags().BoolVarP(&o.NewName, "new", "n", false, "save a new dataset only, using an available name")
  cmd.Flags().BoolVarP(&o.UseDscache, "use-dscache", "", false, "experimental: build and use dscache if none exists")
@@ -87,28 +85,33 @@ type SaveOptions struct {
  Refs *RefSelect
  FilePaths []string
  BodyPath string
- Recall string
  Drop string
 
  Title string
  Message string
 
  Replace bool
  ShowValidation bool
- DryRun bool
- KeepFormat bool
- Force bool
- NoRender bool
- Secrets []string
- NewName bool
- UseDscache bool
+
+ DeprecatedDryRun bool
+
+ KeepFormat bool
+ Force bool
+ NoRender bool
+ Secrets []string
+ NewName bool
+ UseDscache bool
 
  DatasetMethods *lib.DatasetMethods
  FSIMethods *lib.FSIMethods
 }
 
 // Complete adds any missing configuration that can only be added just before calling Run
 func (o *SaveOptions) Complete(f Factory, args []string) (err error) {
+ if o.DeprecatedDryRun {
+ return fmt.Errorf("--dry-run has been removed, use `qri apply` command instead")
+ }
+
  if o.DatasetMethods, err = f.DatasetMethods(); err != nil {
  return
  }
@@ -154,12 +157,9 @@ func (o *SaveOptions) Run() (err error) {
  ScriptOutput: o.ErrOut,
  FilePaths: o.FilePaths,
  Private: false,
- DryRun: o.DryRun,
- Recall: o.Recall,
  Drop: o.Drop,
  ConvertFormatToPrev: o.KeepFormat,
  Force: o.Force,
- ReturnBody: o.DryRun,
  ShouldRender: !o.NoRender,
  NewName: o.NewName,
  UseDscache: o.UseDscache,
@@ -189,13 +189,5 @@ continue?`, true) {
  printWarning(o.ErrOut, fmt.Sprintf("this dataset has %d validation errors", res.Structure.ErrCount))
  }
 
- if o.DryRun {
- data, err := json.MarshalIndent(res, "", " ")
- if err != nil {
- return err
- }
- fmt.Fprint(o.Out, string(data))
- }
-
  return nil
 }

cmd/save_test.go

@@ -128,21 +128,18 @@ func TestSaveRun(t *testing.T) {
  bodypath string
  title string
  message string
- dryrun bool
  noRender bool
  expect string
  err string
  msg string
  }{
- {"no data", "me/bad_dataset", "", "", "", "", false, true, "", "no changes to save", ""},
- {"bad dataset file", "me/cities", "bad/filpath.json", "", "", "", false, true, "", "open bad/filpath.json: no such file or directory", ""},
- {"bad body file", "me/cities", "", "bad/bodypath.csv", "", "", false, true, "", "opening body file: opening dataset.bodyPath 'bad/bodypath.csv': path not found", ""},
- {"good inputs, dryrun", "me/movies", "testdata/movies/dataset.json", "testdata/movies/body_ten.csv", "", "", true, true, "dataset saved: peer/movies\n", "", ""},
- {"good inputs", "me/movies", "testdata/movies/dataset.json", "testdata/movies/body_ten.csv", "", "", false, true, "dataset saved: peer/movies@/mem/QmT7w7Lr2macJ33NA1aiPyCSpM4vPrNUuo4xGdGzwsmL6J\nthis dataset has 1 validation errors\n", "", ""},
- {"add rows, dry run", "me/movies", "testdata/movies/dataset.json", "testdata/movies/body_twenty.csv", "Added 10 more rows", "Adding to the number of rows in dataset", true, true, "dataset saved: peer/movies\n", "", ""},
- {"add rows, save", "me/movies", "testdata/movies/dataset.json", "testdata/movies/body_twenty.csv", "Added 10 more rows", "Adding to the number of rows in dataset", false, true, "dataset saved: peer/movies@/mem/QmTb4ZF9igbKz7ir6b9bbpBvqH7zAsWC1j2h8aaijzjQGA\nthis dataset has 1 validation errors\n", "", ""},
- {"no changes", "me/movies", "testdata/movies/dataset.json", "testdata/movies/body_twenty.csv", "trying to add again", "hopefully this errors", false, true, "", "error saving: no changes", ""},
- {"add viz", "me/movies", "testdata/movies/dataset_with_viz.json", "", "", "", false, false, "dataset saved: peer/movies@/mem/QmXNfs9TeHN9rpyeUb2aABeTq6NKGhKEj94hjUff3YgkBT\nthis dataset has 1 validation errors\n", "", ""},
+ {"no data", "me/bad_dataset", "", "", "", "", true, "", "no changes to save", ""},
+ {"bad dataset file", "me/cities", "bad/filpath.json", "", "", "", true, "", "open bad/filpath.json: no such file or directory", ""},
+ {"bad body file", "me/cities", "", "bad/bodypath.csv", "", "", true, "", "opening body file: opening dataset.bodyPath 'bad/bodypath.csv': path not found", ""},
+ {"good inputs", "me/movies", "testdata/movies/dataset.json", "testdata/movies/body_ten.csv", "", "", true, "dataset saved: peer/movies@/mem/QmT7w7Lr2macJ33NA1aiPyCSpM4vPrNUuo4xGdGzwsmL6J\nthis dataset has 1 validation errors\n", "", ""},
+ {"add rows, save", "me/movies", "testdata/movies/dataset.json", "testdata/movies/body_twenty.csv", "Added 10 more rows", "Adding to the number of rows in dataset", true, "dataset saved: peer/movies@/mem/QmTb4ZF9igbKz7ir6b9bbpBvqH7zAsWC1j2h8aaijzjQGA\nthis dataset has 1 validation errors\n", "", ""},
+ {"no changes", "me/movies", "testdata/movies/dataset.json", "testdata/movies/body_twenty.csv", "trying to add again", "hopefully this errors", true, "", "error saving: no changes", ""},
+ {"add viz", "me/movies", "testdata/movies/dataset_with_viz.json", "", "", "", false, "dataset saved: peer/movies@/mem/QmXNfs9TeHN9rpyeUb2aABeTq6NKGhKEj94hjUff3YgkBT\nthis dataset has 1 validation errors\n", "", ""},
  }
 
  for _, c := range cases {
@@ -165,7 +162,6 @@ func TestSaveRun(t *testing.T) {
  BodyPath: c.bodypath,
  Title: c.title,
  Message: c.message,
- DryRun: c.dryrun,
  NoRender: c.noRender,
  DatasetMethods: dsm,
  }

lib/datasets.go

@@ -454,14 +454,8 @@ type SaveParams struct {
  // option to make dataset private. private data is not currently implimented,
  // see https://github.com/qri-io/qri/issues/291 for updates
  Private bool
- // run without saving, returning results
- DryRun bool
- // if true, res.Dataset.Body will be a fs.file of the body
- ReturnBody bool
  // if true, convert body to the format of the previous version, if applicable
  ConvertFormatToPrev bool
- // string of references to recall before saving
- Recall string
  // comma separated list of component names to delete before saving
  Drop string
  // force a new commit, even if no changes are detected
@@ -511,7 +505,7 @@ func (m *DatasetMethods) Save(p *SaveParams, res *dataset.Dataset) error {
  }
 
  // If the dscache doesn't exist yet, it will only be created if the appropriate flag enables it.
- if p.UseDscache && !p.DryRun {
+ if p.UseDscache {
  c := m.inst.dscache
  c.CreateNewEnabled = true
  }
@@ -560,9 +554,8 @@ func (m *DatasetMethods) Save(p *SaveParams, res *dataset.Dataset) error {
 
  success := false
  defer func() {
- // if creating a new dataset fails, or we're doing a dry-run on a new dataset
- // we need to remove the dataset
- if isNew && (!success || p.DryRun) {
+ // if creating a new dataset fails, we need to remove the dataset
+ if isNew && !success {
  log.Debugf("removing unused log for new dataset %s", ref)
  ctx, cancel := context.WithTimeout(context.Background(), time.Second*2)
  if err := m.inst.logbook.RemoveLog(ctx, ref); err != nil {
@@ -590,15 +583,6 @@ func (m *DatasetMethods) Save(p *SaveParams, res *dataset.Dataset) error {
  }
  }
 
- if p.Recall != "" {
- recall, err := base.Recall(ctx, m.inst.qfs.DefaultWriteFS(), ref, p.Recall)
- if err != nil {
- return err
- }
- recall.Assign(ds)
- ds = recall
- }
-
  if !p.Force && p.Drop == "" &&
  ds.BodyPath == "" &&
  ds.Body == nil &&
@@ -624,11 +608,6 @@ func (m *DatasetMethods) Save(p *SaveParams, res *dataset.Dataset) error {
  scriptOut := p.ScriptOutput
  secrets := p.Secrets
  r := m.inst.repo
- if p.DryRun {
- str.PrintErr("🏃🏽‍♀️ dry run\n")
- // dry run writes to an ephemeral mapstore
- writeDest = qfs.NewMemFS()
- }
 
  // create a loader so transforms can call `load_dataset`
  // TODO(b5) - add a ResolverMode save parameter and call m.inst.resolverForMode
@@ -644,15 +623,6 @@ func (m *DatasetMethods) Save(p *SaveParams, res *dataset.Dataset) error {
  }
  }
 
- if p.DryRun {
- // Tests expect a that a call to `qri save --dry-run` will still construct a full
- // reference with an IPFS path and Name, etc. This isn't actually a valid reference,
- // since nothing is written to the repo, so relying on this is a bit hacky. But using
- // dry-run to save is going away once `apply` exists, so this is temporary anyway.
- *res = *ds
- return nil
- }
-
  if fsiPath != "" && p.Drop != "" {
  return qrierr.New(fmt.Errorf("cannot drop while FSI-linked"), "can't drop component from a working-directory, delete files instead.")
  }
@@ -681,23 +651,17 @@ func (m *DatasetMethods) Save(p *SaveParams, res *dataset.Dataset) error {
  success = true
 
  // TODO (b5) - this should be integrated into base.SaveDataset
- if fsiPath != "" && !p.DryRun {
+ if fsiPath != "" {
  vi := dsref.ConvertDatasetToVersionInfo(savedDs)
  vi.FSIPath = fsiPath
  if err = repo.PutVersionInfoShim(m.inst.repo, &vi); err != nil {
  return err
  }
  }
 
- if p.ReturnBody {
- if err = base.InlineJSONBody(savedDs); err != nil {
- return err
- }
- }
-
  *res = *savedDs
 
- if fsiPath != "" && !p.DryRun {
+ if fsiPath != "" {
  // Need to pass filesystem here so that we can read the README component and write it
  // properly back to disk.
  if writeErr := fsi.WriteComponents(savedDs, fsiPath, m.inst.repo.Filesystem()); err != nil {

lib/datasets_test.go

@@ -165,71 +165,6 @@ func TestDatasetRequestsForceSave(t *testing.T) {
  }
 }
 
-func TestDatasetRequestsSaveRecallDrop(t *testing.T) {
- t.Skip("TODO(dustmop): Recall will be going away soon, apply will take its place")
- ctx, done := context.WithCancel(context.Background())
- defer done()
-
- node := newTestQriNode(t)
- ref := addNowTransformDataset(t, node)
- inst := NewInstanceFromConfigAndNode(ctx, config.DefaultConfigForTesting(), node)
- m := NewDatasetMethods(inst)
-
- metaOnePath := tempDatasetFile(t, "*-meta.json", &dataset.Dataset{Meta: &dataset.Meta{Title: "an updated title"}})
- metaTwoPath := tempDatasetFile(t, "*-meta-2.json", &dataset.Dataset{Meta: &dataset.Meta{Title: "new title!"}})
- defer func() {
- os.RemoveAll(metaOnePath)
- os.RemoveAll(metaTwoPath)
- }()
-
- res := &dataset.Dataset{}
- err := m.Save(&SaveParams{
- Ref: ref.Alias(),
- FilePaths: []string{metaOnePath},
- ReturnBody: true}, res)
- if err != nil {
- t.Fatal(err.Error())
- }
-
- err = m.Save(&SaveParams{
- Ref: ref.Alias(),
- FilePaths: []string{metaOnePath},
- Recall: "wut"}, res)
- if err == nil {
- t.Fatal("expected bad recall to error")
- }
-
- err = m.Save(&SaveParams{
- Ref: ref.Alias(),
- FilePaths: []string{metaTwoPath},
- Recall: "tf"}, res)
- if err != nil {
- t.Fatal(err)
- }
- if res.Transform == nil {
- t.Error("expected transform to exist on recalled save")
- }
-
- err = m.Save(&SaveParams{
- Ref: ref.Alias(),
- Drop: "wut",
- }, res)
- if err == nil {
- t.Fatal("expected bad recall to error")
- }
-
- err = m.Save(&SaveParams{
- Ref: ref.Alias(),
- Drop: "tf",
- }, res)
- if err != nil {
- t.Fatal("expected bad recall to error")
- }
- if res.Transform != nil {
- t.Error("expected transform be nil")
- }
-}
-
 func TestDatasetRequestsSaveZip(t *testing.T) {
  ctx, done := context.WithCancel(context.Background())
  defer done()