Skip to content
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

To unify the naming of keywords #348

Closed
dutor opened this issue May 10, 2019 · 17 comments
Closed

To unify the naming of keywords #348

dutor opened this issue May 10, 2019 · 17 comments
Labels
type/enhancement Type: make the code neat or more efficient
Milestone

Comments

@dutor
Copy link
Contributor

dutor commented May 10, 2019

Such as

  • Replace OVERWRITE with IF NOT EXISTS
  • In admin queries, use DROP instead of REMOVE, CHANGE instead of SET
  • Replace FIND with LOOKUP
@dutor dutor added the feature label May 10, 2019
@darionyaphet
Copy link
Contributor

Update remove tag to drop tag remove edge to drop edge and remove hosts to drop hosts

@ayyt
Copy link
Contributor

ayyt commented May 14, 2019

I don't think the remove host needs to be changed. Becase it isn't a table. refer mysql 8.0

@boshengchen
Copy link
Contributor

Actually, OVERWRITE is not equal to IF NOT EXISTS

OVERWRITE:
    if (exist) {overwrite data; return successful;}

IF NOT EXISTS:
    if (exist) {return successful;}

@boshengchen
Copy link
Contributor

The IF NOT EXISTS and IF EXISTS only used user sentence at current time, if have a feeling of discord, let me delete it at later. WDYT ?@ALL

@darionyaphet
Copy link
Contributor

I don't think the remove host needs to be changed. Becase it isn't a table. refer mysql 8.0

Does MySQL have the concept of host ? maybe they are not the same @steppenwolfyuetong

@ayyt
Copy link
Contributor

ayyt commented May 15, 2019

I don't think the remove host needs to be changed. Becase it isn't a table. refer mysql 8.0

Does MySQL have the concept of host ? maybe they are not the same @steppenwolfyuetong

Yeah,mysql cluster has add hosts and remove hosts commands. when I When I did this function, I have studied it.

If you change it to dropshost, how do you explain add hosts?

@dangleptr
Copy link
Contributor

For hosts, i think register/unregister is more reasonable. wdyt? @steppenwolfyuetong @darionyaphet

@darionyaphet
Copy link
Contributor

Host is a special structure to represent the storage instance and differ from the other.

register/unregister could explain for join and leave the cluster, it's semantics more suitable.

I like it :) @dangleptr Do you have any suggestions? @dutor

@sherman-the-tank
Copy link
Member

register/unregister +1

@sherman-the-tank sherman-the-tank added type/enhancement Type: make the code neat or more efficient and removed feature labels May 16, 2019
@dutor
Copy link
Contributor Author

dutor commented May 16, 2019

register/unregister -1

@dutor
Copy link
Contributor Author

dutor commented May 16, 2019

Host is a special structure to represent the storage instance and differ from the other.

register/unregister could explain for join and leave the cluster, it's semantics more suitable.

I like it :) @dangleptr Do you have any suggestions? @dutor

I don't see any specialness in Host, please enlighten me. For me, register/unregister have no different meanings with add/drop, neither better nor worse.

@sherman-the-tank
Copy link
Member

Honestly I don't see big YES or NO here. I'll say let's use ADD/DROP HOST. Here are the reasons

  1. They are short, so easy to type
  2. Consistent with ADD/DROP USER

@dangleptr
Copy link
Contributor

dangleptr commented May 23, 2019

Host is a special structure to represent the storage instance and differ from the other.
register/unregister could explain for join and leave the cluster, it's semantics more suitable.
I like it :) @dangleptr Do you have any suggestions? @dutor

I don't see any specialness in Host, please enlighten me. For me, register/unregister have no different meanings with add/drop, neither better nor worse.

Actually, in our system, the "add/remove hosts " command is not really add/remove a host into our cluster. It is just register/unregister it in whitelist. It will be used to filter hosts which send heart beat to Meta service.

So i think the accurate command name for it should be "Register/Unregister host in whitelist" or remove the whitelist concept and trust our users (I vote for it now).

Honestly I don't see big YES or NO here. I'll say let's use ADD/DROP HOST. Here are the reasons

  1. They are short, so easy to type
  2. Consistent with ADD/DROP USER
  1. I don't think there is any difference for typing between DROP and REMOVE.
  2. For USER commands, we use CREATE/DROP now.

@ayyt
Copy link
Contributor

ayyt commented May 23, 2019

I think we can refer to the corresponding sql of the current popular database.

@sherman-the-tank
Copy link
Member

First we need to make the concept clear. We DO NOT have a white list. What we have is called CLUSTER. The host in the cluster might not serve due to all kinds of reason.

So what we want to do here is not creating a new host, we just want to add a new host to the cluster, or remove a host from the cluster. So I think here ADD and REGISTER means the same thing, and REMOVE/DROP/UNREGISTER have the same meaning as well.

So, in order to keep the keywords simple, we can just use ADD/REMOVE, or ADD/DROP

@sherman-the-tank sherman-the-tank added this to the v1_beta_release milestone May 30, 2019
@whitewum
Copy link
Contributor

hi everyone, do you have any conclusions about the namings?
what about the IF EXISTS or IF NOT EXISTS?

@sherman-the-tank
Copy link
Member

Done

yixinglu added a commit to yixinglu/nebula that referenced this issue Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Type: make the code neat or more efficient
Projects
None yet
Development

No branches or pull requests

7 participants