More PR feedback and cleanup, bug fixes

go/libraries/doltcore/dbfactory/factory.go

@@ -52,9 +52,12 @@ const (
 	defaultMemTableSize = 256 * 1024 * 1024
 )
 
-// DBFactory is an interface for creating concrete datas.Database instances which may have different backing stores.
+// DBFactory is an interface for creating concrete datas.Database instances from different backing stores
 type DBFactory interface {
+	// CreateDB returns the database located at the URL given and its associated data access interfaces
 	CreateDB(ctx context.Context, nbf *types.NomsBinFormat, u *url.URL, params map[string]interface{}) (datas.Database, types.ValueReadWriter, tree.NodeStore, error)
+	// PrepareDB does any necessary setup work for a new database to be created at the URL given, e.g. to receive a push.
+	// Not all factories support this operation.
 	PrepareDB(ctx context.Context, nbf *types.NomsBinFormat, u *url.URL, params map[string]interface{}) error
 }
 

go/libraries/doltcore/dbfactory/gs.go

@@ -16,7 +16,6 @@ package dbfactory
 
 import (
 	"context"
-	"fmt"
 	"net/url"
 	"path/filepath"
 
@@ -67,7 +66,8 @@ type LocalBSFactory struct {
 }
 
 func (fact LocalBSFactory) PrepareDB(ctx context.Context, nbf *types.NomsBinFormat, urlObj *url.URL, params map[string]interface{}) error {
-	return fmt.Errorf("bs scheme cannot support this operation")
+	// nothing to prepare
+	return nil
 }
 
 // CreateDB creates a local filesystem blobstore backed database

go/libraries/doltcore/dbfactory/mem.go

@@ -16,7 +16,6 @@ package dbfactory
 
 import (
 	"context"
-	"fmt"
 	"net/url"
 
 	"github.com/dolthub/dolt/go/store/chunks"
@@ -30,7 +29,8 @@ type MemFactory struct {
 }
 
 func (fact MemFactory) PrepareDB(ctx context.Context, nbf *types.NomsBinFormat, urlObj *url.URL, params map[string]interface{}) error {
-	return fmt.Errorf("memory scheme cannot support this operation")
+	// nothing to prepare
+	return nil
 }
 
 // CreateDB creates an in memory backed database

go/libraries/doltcore/sqle/database_provider.go

@@ -211,9 +211,14 @@ func (p DoltDatabaseProvider) attemptCloneReplica(ctx *sql.Context, dbName strin
 		return nil
 	}
 
+	urlTemplate, ok := remoteUrlTemplate.(string)
+	if !ok {
+		return nil
+	}
+
 	// TODO: url sanitize
 	// TODO: SQL identifiers aren't case sensitive, but URLs are, need a plan for this
-	remoteUrl := fmt.Sprintf(remoteUrlTemplate.(string), dbName)
+	remoteUrl := strings.Replace(urlTemplate, dsess.URLTemplateDatabasePlaceholder, dbName, -1)
 
 	// TODO: remote params for AWS, others
 	// TODO: this needs to be robust in the face of the DB not having the default branch
@@ -369,8 +374,8 @@ func (p DoltDatabaseProvider) configureReplication(ctx *sql.Context, name string
 		return nil
 	}
 
-	// TODO: url sanitize
-	remoteUrl := strings.Replace(urlTemplate, dsess.URLTemplateDatabasePlaceholder, name, 1)
+	// TODO: url sanitize name 
+	remoteUrl := strings.Replace(urlTemplate, dsess.URLTemplateDatabasePlaceholder, name, -1)
 
 	// TODO: params for AWS, others that need them
 	r := env.NewRemote(remoteName, remoteUrl, nil)

integration-tests/bats/replication-multidb.bats

@@ -208,7 +208,7 @@ SQL
     # mess
     dolt config --global --unset sqlserver.global.dolt_replicate_to_remote
     dolt config --global --add sqlserver.global.dolt_read_replica_remote remote1
-    dolt config --global --add sqlserver.global.dolt_read_replica_remote.dolt_replicate_all_heads 1
+    dolt config --global --add sqlserver.global.dolt_replicate_all_heads 1
 
     [ ! -d "dbs2/newdb" ]