support removing table_id/store_id from namespace. #776
Conversation
dantin
commented
Sep 28, 2017
dantin
commented
- add info log
- add function to remove table_id/store_id from namespace
|
Hi contributor, thanks for your PR. This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically. |
|
/ok-to-test |
pdctl/command/namespace_command.go
Outdated
|
|
||
| postJSON(cmd, namespaceAddPrefix, input) | ||
| postJSON(cmd, namespaceRelPrefix, input) |
There was a problem hiding this comment.
Why use another URL as you have defined different action fields?
There was a problem hiding this comment.
The old URL may confuse user as it also support 'delete' function.
server/api/namespace.go
Outdated
| if err := cluster.AddNamespaceTableID(ns, tableID); err != nil { | ||
| h.rd.JSON(w, http.StatusInternalServerError, err.Error()) | ||
| switch action { | ||
| case "append": |
There was a problem hiding this comment.
How about add instead of append?
pdctl/command/namespace_command.go
Outdated
| return | ||
| } | ||
|
|
||
| input := make(map[string]interface{}) |
There was a problem hiding this comment.
input := map[string]interface{} {
"namespace" : args[0],
......
}may be better.
pdctl/command/store_command.go
Outdated
| @@ -30,14 +30,15 @@ var ( | |||
| // NewStoreCommand return a store subcommand of rootCmd | |||
| func NewStoreCommand() *cobra.Command { | |||
| s := &cobra.Command{ | |||
| Use: "store [delete|label|weight] <store_id>", | |||
| Use: "store [delete|label|weight|set_namespace|rm_namespace] <store_id> [namespace]", | |||
There was a problem hiding this comment.
I prefer using another subcommand here, not in store. maybe store_ns set|rm
There was a problem hiding this comment.
OK, I'll move it to another subcommand.
pdctl/command/namespace_command.go
Outdated
| @@ -23,18 +23,19 @@ import ( | |||
|
|
|||
| const ( | |||
| namespacePrefix = "pd/api/v1/namespaces" | |||
| namespaceAddPrefix = "pd/api/v1/namespaces/add" | |||
| namespaceRelPrefix = "pd/api/v1/namespaces/rel" | |||
There was a problem hiding this comment.
I am confused what dos rel mean here?
There was a problem hiding this comment.
What I means here is relationship. @disksing raised the same question, maybe we need a new name to make it more clear.
There was a problem hiding this comment.
I think you can use /namespaces/table here.
|
PTAL @disksing |
|
LGTM |
|
LGTM. |
