-
-
Notifications
You must be signed in to change notification settings - Fork 480
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bh/clone2 #83
Conversation
@@ -83,42 +83,45 @@ func main() { | |||
func runMain() int { | |||
args := os.Args[1:] | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this isn't really part of this feature. Just cleanup of debug flag processing.
doneChan := make(chan struct{}) | ||
go progFunc(progChan, doneChan) | ||
for _, brnch := range branches { | ||
branch = brnch.GetPath() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you're just taking the last branch in the case that there isn't a master? Intended behavior?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
matches previous behavior
return hash.New(resp.RootHash), tblFiles, nil | ||
} | ||
|
||
// SetRootChunk sets the root chunk changes the root chunk hash from the previous value to the new root. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
@@ -35,6 +35,8 @@ service ChunkStoreService { | |||
rpc Root(RootRequest) returns (RootResponse); | |||
|
|||
rpc Commit(CommitRequest) returns (CommitResponse); | |||
|
|||
rpc EnumerateTables(EnumerateTablesRequest) returns (EnumerateTablesResponse); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
List!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
panic("Not Implemented") | ||
} | ||
|
||
// DoltRemoteTableFile is an implementation of a TableFile that live in a DoltChunkStore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lives
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
@@ -123,3 +123,16 @@ func (c RetryingChunkStoreServiceClient) Commit(ctx context.Context, in *remotes | |||
|
|||
return resp, err | |||
} | |||
|
|||
func (c RetryingChunkStoreServiceClient) EnumerateTables(ctx context.Context, in *remotesapi.EnumerateTablesRequest, opts ...grpc.CallOption) (*remotesapi.EnumerateTablesResponse, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget this reference when you rename the proto method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
global replace :)
go/store/nbs/store.go
Outdated
return NomsBlockStoreTableSink{fileId, numChunks, f, nbs}, nil | ||
} | ||
|
||
// SetRootChunk sets the root chunk changes the root chunk hash from the previous value to the new root. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same typo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
|
||
func TestNBSAsTableFileStore(t *testing.T) { | ||
ctx := context.Background() | ||
testDir := filepath.Join(os.TempDir(), uuid.New().String()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ioutil has a better way of making a randomly named temp dir for tests
|
||
if err != nil { | ||
logger("failed to write data to response. err : " + err.Error()) | ||
return -1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be an HTTP code?
return -1 | ||
} | ||
|
||
return -1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be http.ok?
go/cmd/dolt/commands/clone.go
Outdated
} else { | ||
var err error | ||
branches, err = srcDB.GetBranches(ctx) | ||
err := actions.Clone(ctx, srcDB, dEnv.DoltDB) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean we lose progress indicators?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
This version of clone works on the table files directly. It enumerates all the table files and downloads them. It does not inspect the chunks as v1 did.