search

goraft / raft

UNMAINTAINED: A Go implementation of the Raft distributed consensus protocol.
MIT License
2.43k stars 480 forks source link

fix(server.Init) Fix wrong error reporting #203

Closed xiang90 closed 10 years ago

xiang90 commented 10 years ago

We should only report error if the snapshot path is not exist and we cannot create a new one.

xiang90 commented 10 years ago

/cc @benbjohnson https://drone.io/github.com/xiangli-cmu/raft/24

Introduced a bug in last pr :(

philips commented 10 years ago

dang, lgtm.

benbjohnson commented 10 years ago

@xiangli-cmu What happens if the directory exists but it's not writable? I don't quite understand why we're only checking for one type of error with Mkdir().

xiang90 commented 10 years ago

we are checking for all types except previous existence. pervious existence is not an error we should report.

benbjohnson commented 10 years ago

@xiangli-cmu How is err != nil && os.IsNotExist(err) checking for all types except for existence? That should only be true where err == ErrNotExist. Basically, only when the directory doesn't exist.

Did you mean err != nil && !os.IsNotExist(err)?

http://golang.org/pkg/os/#IsNotExist

xiang90 commented 10 years ago

@benbjohnson 

IsNotExist returns a boolean indicating whether the error is known to report that a file or directory does not exist. It is satisfied by ErrNotExist as well as some syscall errors.

It says "It is satisfied by ErrNotExist as well as some syscall errors". Did I misunderstand this sentence?

benbjohnson commented 10 years ago

@xiangli-cmu The only time ErrNotExists will occur with Mkdir() is if the parent directory doesn't exist. In this case, the parent directory is s.path, which should exist.

http://play.golang.org/p/Kezcz9swTX

xiang90 commented 10 years ago

hm... i misunderstood this error sorry

@xiangli-cmu https://github.com/xiangli-cmu The only time ErrNotExistswill occur with Mkdir() is if the parent directory doesn't exist. In this case, the parent directory is s.path, which should exist.

http://play.golang.org/p/Kezcz9swTX

Reply to this email directly or view it on GitHubhttps://github.com/goraft/raft/pull/203#issuecomment-38364784 .

benbjohnson commented 10 years ago

No worries. You just need a bang for all errors besides "not exists": err != nil && !os.IsNotExist(err)