diff --git a/api/datasets.go b/api/datasets.go index 730a38740..32725e8a7 100644 --- a/api/datasets.go +++ b/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, diff --git a/api/datasets_test.go b/api/datasets_test.go index 6358f02e8..4bde60c58 100644 --- a/api/datasets_test.go +++ b/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", }, }, } diff --git a/api/testdata/api.snapshot b/api/testdata/api.snapshot index 823b2073c..f00622615 100755 Binary files a/api/testdata/api.snapshot and b/api/testdata/api.snapshot differ diff --git a/base/revisions.go b/base/revisions.go index 28d254802..246e17955 100644 --- a/base/revisions.go +++ b/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 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) { diff --git a/base/revisions_test.go b/base/revisions_test.go index 405102171..2ad2c82e2 100644 --- a/base/revisions_test.go +++ b/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) diff --git a/cmd/save.go b/cmd/save.go index c35f15e65..bfae95092 100644 --- a/cmd/save.go +++ b/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,7 +85,6 @@ type SaveOptions struct { Refs *RefSelect FilePaths []string BodyPath string - Recall string Drop string Title string @@ -95,13 +92,15 @@ type SaveOptions struct { 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 @@ -109,6 +108,10 @@ type SaveOptions struct { // 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 } diff --git a/cmd/save_test.go b/cmd/save_test.go index 6ffdc8df8..5b3bc4af4 100644 --- a/cmd/save_test.go +++ b/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, } diff --git a/lib/datasets.go b/lib/datasets.go index f7aadffa7..48ba4d389 100644 --- a/lib/datasets.go +++ b/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,7 +651,7 @@ 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 { @@ -689,15 +659,9 @@ func (m *DatasetMethods) Save(p *SaveParams, res *dataset.Dataset) error { } } - 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 { diff --git a/lib/datasets_test.go b/lib/datasets_test.go index 1e19eeaf7..bb8999f9a 100644 --- a/lib/datasets_test.go +++ b/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() diff --git a/lib/lib_test.go b/lib/lib_test.go index 0d633b256..9cde625eb 100644 --- a/lib/lib_test.go +++ b/lib/lib_test.go @@ -229,23 +229,6 @@ func saveDataset(ctx context.Context, r repo.Repo, ds *dataset.Dataset, sw base. return dsref.ConvertDatasetToVersionInfo(res).SimpleRef(), nil } -func addNowTransformDataset(t *testing.T, node *p2p.QriNode) dsref.Ref { - ctx := context.Background() - tc, err := dstest.NewTestCaseFromDir("testdata/now_tf") - if err != nil { - t.Fatal(err.Error()) - } - ds := tc.Input - ds.Name = tc.Name - ds.Transform.ScriptPath = "testdata/now_tf/transform.star" - - ref, err := saveDataset(ctx, node.Repo, ds, base.SaveSwitches{Pin: true}) - if err != nil { - t.Fatal(err.Error()) - } - return ref -} - func TestInstanceEventSubscription(t *testing.T) { timeoutMs := time.Millisecond * 250 if runtime.GOOS == "windows" {