Automatically push and pull new databases in replicated servers #4276
Conversation
|
|
||
| // TODO: url sanitize | ||
| // TODO: SQL identifiers aren't case sensitive, but URLs are, need a plan for this | ||
| remoteUrl := fmt.Sprintf(remoteUrlTemplate.(string), dbName) |
There was a problem hiding this comment.
Right now the URL template has a printf marker (%s) in it, but that's probably not what we want.
Maybe $database$? Thoughts?
There was a problem hiding this comment.
{database}? \(database) (swift syntax...)?
All seem decently safe...
There was a problem hiding this comment.
I went with {database}, thanks!
| @@ -28,6 +29,10 @@ import ( | |||
| 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") | |||
There was a problem hiding this comment.
Spoke offline, I don't think these factories should return an error here. MemFactory definitely supports this use case, remotesapi supports it in some cases (remotesrv, dolthub repos that already exist, etc).
go/libraries/doltcore/env/remotes.go
Outdated
| urlStr = strings.ReplaceAll(urlStr, `\`, "/") | ||
| if !strings.HasPrefix(urlStr, "/") { | ||
| if !strings.HasPrefix(urlStr, "/") && !isDriveLetterPath(urlStr) { | ||
| urlStr = "/" + urlStr | ||
| } |
There was a problem hiding this comment.
This looks sketchy to me. We already made the path absolute up above why would we ever need to prefix a /?
Should this just be urlStr = filepath.ToSlash(urlStr), and then using the thing should also filepath.FromSlash()?
There was a problem hiding this comment.
You might be right that this code is useless and was only ever running on windows. I'll try removing it and see where it gets us.
|
|
||
| // TODO: url sanitize | ||
| // TODO: SQL identifiers aren't case sensitive, but URLs are, need a plan for this | ||
| remoteUrl := fmt.Sprintf(remoteUrlTemplate.(string), dbName) |
There was a problem hiding this comment.
{database}? \(database) (swift syntax...)?
All seem decently safe...
No description provided.