feat add dir_flag #376
feat add dir_flag #376
Conversation
| @@ -59,6 +60,7 @@ func init() { | |||
| errors[EcodeNodeExist] = "Already exists" // create | |||
| errors[EcodeRootROnly] = "Root is read only" | |||
| errors[EcodeKeyIsPreserved] = "The prefix of given key is a keyword in etcd" | |||
| errors[EcodeDirNotEmpty] = "The directory is not empty" | |||
There was a problem hiding this comment.
Lets stay with unix tradition "Directory not empty"
|
@philips I will add tests. |
| return etcdErr.NewError(etcdErr.EcodeNotFile, "", n.store.Index()) | ||
| if n.IsDir() { | ||
| if !dir { | ||
| // cannot delete a directory without set recursive to true |
There was a problem hiding this comment.
"without recursive set to true"
|
lgtm, this will break the store from older versions of etcd, right? |
|
cleanup the small comments and lets merge this! |
|
@philips It will break the store state when replay the log from previous version. |
|
We need to fix the log replay then. @benbjohnson can you look at making
|
|
@philips The delete part do not have any problem since we just add a flag. For create/set, empty value used to create a directory but now it will create a key with empty value. |
|
Right but we want to be able to move logs from v1
|
|
@philips v1 to v2 should not be a problem. I think we just broke rc version, which we are not guarantee to support upgrade if I remember correctly. I will be great if @benbjohnson can help to test v1 to v2. |
|
@xiangli-cmu There's an existing v1 migration test: https://github.com/coreos/etcd/blob/master/tests/functional/v1_migration_test.go It takes the log generated from a v1 binary and loads it into a v2 binary. What else do you need? |
|
@benbjohnson This change does not break the test. So it proves v1 should be able to migrate to v2 without a problem? |
|
@xiangli-cmu @benbjohnson I just don't know if it actually works anymore after this change. I assumed it would not and the travis build is currently broken. If it is all working fine then great! :) |
|
@xiangli-cmu It doesn't look like the migration test has directory creation in it. I agree with @philips that this change probably breaks v1 logs. We probably need to duplicate the |
|
@benbjohnson You cannot create a directory in v1. You can only implicitly create directory by creating keys. That is why it probably does not break v1. |
|
@xiangli-cmu Ok, I didn't realize that. Yeah, in that case it probably shouldn't break. |
for create/delete a directory, dir=ture will be required.
recursive=true will imply dir=true
empty value of a key-value pair will be allowed.