feat(logbook): write push & pull operations, add logbook summary output format

Merge pull request #1426 from qri-io/log_populate_published

Author: b5

base/log_test.go

@@ -92,7 +92,7 @@ func TestDatasetLogForeign(t *testing.T) {
 	ref.Path = "QmExample"
 	foreignBuilder.Commit(ctx, t, initID, "their commit", ref.Path)
 	foreignBook := foreignBuilder.Logbook()
-	foreignLog, err := foreignBook.UserDatasetRef(ctx, ref)
+	foreignLog, err := foreignBook.UserDatasetBranchesLog(ctx, initID)
 	if err != nil {
 		t.Fatal(err)
 	}

cmd/log.go

@@ -159,15 +159,17 @@ The logbook command shows entries for a dataset, from newest to oldest.`,
 			}
 			if o.Raw {
 				return o.RawLogs()
+			} else if o.Summary {
+				return o.LogbookSummary()
 			}
 			return o.Logbook()
 		},
 	}
 
-	// cmd.Flags().StringVarP(&o.Format, "format", "f", "", "set output format [json]")
 	cmd.Flags().IntVar(&o.PageSize, "page-size", 25, "page size of results, default 25")
 	cmd.Flags().IntVar(&o.Page, "page", 1, "page number of results, default 1")
 	cmd.Flags().BoolVar(&o.Raw, "raw", false, "full logbook in raw JSON format. overrides all other flags")
+	cmd.Flags().BoolVar(&o.Summary, "summary", false, "print one oplog per line in the format 'MODEL ID OPCOUNT NAME'. overrides all other flags")
 
 	return cmd
 }
@@ -176,17 +178,21 @@ The logbook command shows entries for a dataset, from newest to oldest.`,
 type LogbookOptions struct {
 	ioes.IOStreams
 
-	PageSize int
-	Page     int
-	Refs     *RefSelect
-	Raw      bool
+	PageSize     int
+	Page         int
+	Refs         *RefSelect
+	Raw, Summary bool
 
 	LogMethods *lib.LogMethods
 }
 
 // Complete adds any missing configuration that can only be added just before calling Run
 func (o *LogbookOptions) Complete(f Factory, args []string) (err error) {
-	if o.Raw {
+	if o.Raw && o.Summary {
+		return fmt.Errorf("cannot use summary & raw flags at once")
+	}
+
+	if o.Raw || o.Summary {
 		if len(args) != 0 {
 			return fmt.Errorf("can't use dataset reference. the raw flag shows the entire logbook")
 		}
@@ -248,3 +254,14 @@ func (o *LogbookOptions) RawLogs() error {
 	printToPager(o.Out, bytes.NewBuffer(data))
 	return nil
 }
+
+// LogbookSummary prints a logbook overview
+func (o *LogbookOptions) LogbookSummary() error {
+	var res string
+	if err := o.LogMethods.LogbookSummary(&struct{}{}, &res); err != nil {
+		return err
+	}
+
+	printToPager(o.Out, bytes.NewBufferString(res))
+	return nil
+}

cmd/save.go

@@ -67,7 +67,6 @@ commit message and title to the save.`,
 	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().BoolVarP(&o.Publish, "publish", "p", false, "publish this dataset to the registry")
 	cmd.Flags().BoolVar(&o.DryRun, "dry-run", false, "simulate saving a dataset")
 	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")
@@ -95,7 +94,6 @@ type SaveOptions struct {
 
 	Replace        bool
 	ShowValidation bool
-	Publish        bool
 	DryRun         bool
 	KeepFormat     bool
 	Force          bool
@@ -158,7 +156,6 @@ func (o *SaveOptions) Run() (err error) {
 		ScriptOutput:        o.ErrOut,
 		FilePaths:           o.FilePaths,
 		Private:             false,
-		Publish:             o.Publish,
 		DryRun:              o.DryRun,
 		Recall:              o.Recall,
 		Drop:                o.Drop,

cmd/save_test.go

@@ -126,22 +126,21 @@ func TestSaveRun(t *testing.T) {
 		bodypath    string
 		title       string
 		message     string
-		publish     bool
 		dryrun      bool
 		noRender    bool
 		expect      string
 		err         string
 		msg         string
 	}{
-		{"no data", "me/bad_dataset", "", "", "", "", false, false, true, "", "no changes to save", ""},
-		{"bad dataset file", "me/cities", "bad/filpath.json", "", "", "", false, false, true, "", "open bad/filpath.json: no such file or directory", ""},
-		{"bad body file", "me/cities", "", "bad/bodypath.csv", "", "", false, false, true, "", "opening dataset.bodyPath 'bad/bodypath.csv': path not found", ""},
-		{"good inputs, dryrun", "me/movies", "testdata/movies/dataset.json", "testdata/movies/body_ten.csv", "", "", false, true, true, "dataset saved: peer/movies\n", "", ""},
-		{"good inputs", "me/movies", "testdata/movies/dataset.json", "testdata/movies/body_ten.csv", "", "", true, false, true, "dataset saved: peer/movies@/map/QmVDSrf4BpupXYmDJc6eXanFVW9DS6g3Pfkkx6YqkcUjEQ\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", false, 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", true, false, true, "dataset saved: peer/movies@/map/QmZof1Gc95VFxaAXBq28pTYLe4nQ3TMi4ceBL7ExDkXyyj\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, false, true, "", "error saving: no changes", ""},
-		{"add viz", "me/movies", "testdata/movies/dataset_with_viz.json", "", "", "", false, false, false, "dataset saved: peer/movies@/map/QmXDJLrEePbBGro5m9K9edszqN89XMePrampGhBq7QkyoF\nthis dataset has 1 validation errors\n", "", ""},
+		{"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 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@/map/QmVDSrf4BpupXYmDJc6eXanFVW9DS6g3Pfkkx6YqkcUjEQ\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@/map/QmZof1Gc95VFxaAXBq28pTYLe4nQ3TMi4ceBL7ExDkXyyj\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@/map/QmXDJLrEePbBGro5m9K9edszqN89XMePrampGhBq7QkyoF\nthis dataset has 1 validation errors\n", "", ""},
 	}
 
 	for _, c := range cases {
@@ -164,7 +163,6 @@ func TestSaveRun(t *testing.T) {
 			BodyPath:       c.bodypath,
 			Title:          c.title,
 			Message:        c.message,
-			Publish:        c.publish,
 			DryRun:         c.dryrun,
 			NoRender:       c.noRender,
 			DatasetMethods: dsm,

dsref/spec/resolve.go

@@ -208,9 +208,7 @@ func GenerateExampleOplog(ctx context.Context, journal *logbook.Book, dsname, he
 		return "", nil, err
 	}
 
-	// TODO (b5) - we need UserDatasetRef here b/c it returns the full hierarchy
-	// of oplogs. This method should take an InitID
-	lg, err := journal.UserDatasetRef(ctx, dsref.Ref{Username: username, Name: dsname})
+	lg, err := journal.UserDatasetBranchesLog(ctx, initID)
 	if err != nil {
 		return "", nil, err
 	}

lib/datasets.go

@@ -434,8 +434,6 @@ 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
-	// if true, set saved dataset to published
-	Publish bool
 	// run without saving, returning results
 	DryRun bool
 	// if true, res.Dataset.Body will be a fs.file of the body
@@ -725,23 +723,6 @@ func (m *DatasetMethods) Save(p *SaveParams, res *reporef.DatasetRef) error {
 		}
 	}
 
-	if p.Publish {
-		if p.DryRun {
-			return fmt.Errorf("can't use publish & dry-run together")
-		}
-		var publishedRef reporef.DatasetRef
-		err = m.SetPublishStatus(&SetPublishStatusParams{
-			Ref:           datasetRef.String(),
-			PublishStatus: true,
-			// UpdateRegistry:    true,
-			// UpdateRegistryPin: true,
-		}, &publishedRef)
-
-		if err != nil {
-			return err
-		}
-	}
-
 	*res = datasetRef
 
 	if fsiPath != "" && !p.DryRun {
@@ -770,37 +751,6 @@ func (m *DatasetMethods) nameIsInUse(ctx context.Context, ref dsref.Ref) bool {
 	return true
 }
 
-// SetPublishStatusParams encapsulates parameters for setting the publication status of a dataset
-type SetPublishStatusParams struct {
-	Ref           string
-	PublishStatus bool
-	// UpdateRegistry    bool
-	// UpdateRegistryPin bool
-}
-
-// SetPublishStatus updates the publicity of a reference in the peer's namespace
-func (m *DatasetMethods) SetPublishStatus(p *SetPublishStatusParams, publishedRef *reporef.DatasetRef) error {
-	if m.inst.rpc != nil {
-		return checkRPCError(m.inst.rpc.Call("DatasetMethods.SetPublishStatus", p, publishedRef))
-	}
-
-	ref, err := repo.ParseDatasetRef(p.Ref)
-	if err != nil {
-		return err
-	}
-	if err = repo.CanonicalizeDatasetRef(m.inst.repo, &ref); err != nil {
-		return err
-	}
-
-	ref.Published = p.PublishStatus
-	if err = base.SetPublishStatus(m.inst.repo, &ref, ref.Published); err != nil {
-		return err
-	}
-
-	*publishedRef = ref
-	return nil
-}
-
 // RenameParams defines parameters for Dataset renaming
 type RenameParams struct {
 	Current, Next string
@@ -1059,16 +1009,9 @@ func (m *DatasetMethods) Add(p *AddParams, res *reporef.DatasetRef) error {
 		p.RemoteAddr = m.inst.cfg.Registry.Location
 	}
 
-	mergeLogsError := m.inst.remoteClient.CloneLogs(ctx, ref, p.RemoteAddr)
-	// TODO(b5) - this line is swallowing errors that the cmd package integration
-	// tests are hitting. We need to change the behaviour of add to *require* logs
-	// successfully merge, which will require fixing lots of tests in cmd,
-	// ideally by removing remote.MockRepoClient
-	if p.LogsOnly {
-		return mergeLogsError
-	}
-	if mergeLogsError != nil {
-		return mergeLogsError
+	if err := m.inst.remoteClient.CloneLogs(ctx, ref, p.RemoteAddr); err != nil {
+		log.Error(err)
+		return err
 	}
 
 	rref := reporef.RefFromDsref(ref)

lib/integration_test.go

@@ -132,6 +132,17 @@ func TestReferencePulling(t *testing.T) {
 	ref := InitWorldBankDataset(t, nasim)
 	PublishToRegistry(t, nasim, ref.AliasString())
 
+	// - nasim's local repo should reflect publication
+	logRes := []DatasetLogItem{}
+	err := NewLogMethods(nasim).Log(&LogParams{Ref: ref.AliasString(), ListParams: ListParams{Limit: 1}}, &logRes)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if logRes[0].Published != true {
+		t.Errorf("nasim has published HEAD. ref[0] published is false")
+	}
+
 	hinshun := tr.InitHinshun(t)
 	sqlm := NewSQLMethods(hinshun)
 
@@ -162,7 +173,7 @@ func TestReferencePulling(t *testing.T) {
 	dsm := NewDatasetMethods(adnan)
 
 	// run a transform script that relies on world_bank_population, which adnan's
-	// node should automatically pull to execute thisscript
+	// node should automatically pull to execute this script
 	tfScriptData := `
 wbp = load_dataset("nasim/world_bank_population")
 
@@ -185,6 +196,17 @@ def transform(ds, ctx):
 	if err := dsm.Save(saveParams, res); err != nil {
 		t.Fatal(err)
 	}
+
+	// - adnan's local repo should reflect nasim's publication
+	logRes = []DatasetLogItem{}
+	err = NewLogMethods(adnan).Log(&LogParams{Ref: ref.AliasString(), ListParams: ListParams{Limit: 1}}, &logRes)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if logRes[0].Published != true {
+		t.Errorf("adnan's log expects head was published, ref[0] published is false")
+	}
 }
 
 type NetworkIntegrationTestRunner struct {
@@ -364,8 +386,7 @@ func AssertLogsEqual(a, b *Instance, ref *reporef.DatasetRef) error {
 func InitWorldBankDataset(t *testing.T, inst *Instance) *reporef.DatasetRef {
 	res := &reporef.DatasetRef{}
 	err := NewDatasetMethods(inst).Save(&SaveParams{
-		Publish: true,
-		Ref:     "me/world_bank_population",
+		Ref: "me/world_bank_population",
 		Dataset: &dataset.Dataset{
 			Meta: &dataset.Meta{
 				Title: "World Bank Population",
@@ -390,8 +411,7 @@ d,e,f,false,3`),
 func Commit2WorldBank(t *testing.T, inst *Instance) *reporef.DatasetRef {
 	res := &reporef.DatasetRef{}
 	err := NewDatasetMethods(inst).Save(&SaveParams{
-		Publish: true,
-		Ref:     "me/world_bank_population",
+		Ref: "me/world_bank_population",
 		Dataset: &dataset.Dataset{
 			Meta: &dataset.Meta{
 				Title: "World Bank Population",

lib/log.go

@@ -114,10 +114,6 @@ func (m *LogMethods) Log(params *LogParams, res *[]DatasetLogItem) error {
 		}
 	}
 
-	// TODO (b5) - store logs on pull
-	// if params.Pull {
-	// }
-
 	*res = items
 	return nil
 }
@@ -163,12 +159,21 @@ type PlainLogsParams struct {
 type PlainLogs = []logbook.PlainLog
 
 // PlainLogs encodes the full logbook as human-oriented json
-func (m *LogMethods) PlainLogs(p *PlainLogsParams, res *PlainLogs) error {
-	var err error
+func (m *LogMethods) PlainLogs(p *PlainLogsParams, res *PlainLogs) (err error) {
 	if m.inst.rpc != nil {
 		return checkRPCError(m.inst.rpc.Call("LogMethods.PlainLogs", p, res))
 	}
 	ctx := context.TODO()
 	*res, err = m.inst.repo.Logbook().PlainLogs(ctx)
 	return err
 }
+
+// LogbookSummary returns a string overview of the logbook
+func (m *LogMethods) LogbookSummary(p *struct{}, res *string) (err error) {
+	if m.inst.rpc != nil {
+		return checkRPCError(m.inst.rpc.Call("LogMethods.Diagnostic", p, res))
+	}
+	ctx := context.TODO()
+	*res = m.inst.repo.Logbook().SummaryString(ctx)
+	return nil
+}

lib/remote.go

@@ -45,14 +45,18 @@ func (r *RemoteMethods) Publish(p *PublicationParams, res *dsref.Ref) error {
 		return checkRPCError(r.inst.rpc.Call("RemoteMethods.Publish", p, res))
 	}
 
-	ref, err := repo.ParseDatasetRef(p.Ref)
+	// TODO (b5) - need contexts yo
+	ctx := context.TODO()
+
+	ref, err := dsref.Parse(p.Ref)
 	if err != nil {
 		return err
 	}
 	if ref.Path != "" {
 		return fmt.Errorf("can only publish entire dataset, cannot use version %s", ref.Path)
 	}
-	if err = repo.CanonicalizeDatasetRef(r.inst.Repo(), &ref); err != nil {
+
+	if _, err := r.inst.ResolveReference(ctx, &ref, "local"); err != nil {
 		return err
 	}
 
@@ -61,27 +65,26 @@ func (r *RemoteMethods) Publish(p *PublicationParams, res *dsref.Ref) error {
 		return err
 	}
 
-	// TODO (b5) - need contexts yo
-	ctx := context.TODO()
-
-	// TODO (b5) - we're early in log syncronization days. This is going to fail a bunch
-	// while we work to upgrade the stack. Long term we may want to consider a mechanism
-	// for allowing partial completion where only one of logs or dataset pushing works
-	// by doing both in parallel and reporting issues on both
-	if pushLogsErr := r.inst.RemoteClient().PushLogs(ctx, reporef.ConvertToDsref(ref), addr); pushLogsErr != nil {
-		log.Errorf("pushing logs: %s", pushLogsErr)
+	if err = r.inst.RemoteClient().PushLogs(ctx, ref, addr); err != nil {
+		return err
 	}
 
-	if err = r.inst.RemoteClient().PushDataset(ctx, ref, addr); err != nil {
+	// TODO (b5) - another example of where reference resolution needs to set
+	// the profileID, and isn't. the conversion to a reporef & dataset push will
+	// fail if profileID isn't specified
+	pro, _ := r.inst.Repo().Profile()
+	ref.ProfileID = pro.ID.String()
+
+	datasetRef := reporef.RefFromDsref(ref)
+	if err = r.inst.RemoteClient().PushDataset(ctx, datasetRef, addr); err != nil {
 		return err
 	}
-
-	ref.Published = true
-	if err = base.SetPublishStatus(r.inst.node.Repo, &ref, ref.Published); err != nil {
+	datasetRef.Published = true
+	if err = base.SetPublishStatus(r.inst.node.Repo, &datasetRef, datasetRef.Published); err != nil {
 		return err
 	}
 
-	*res = reporef.ConvertToDsref(ref)
+	*res = ref
 	return nil
 }