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

support watch resource group #1035

Merged
merged 4 commits into from
Jan 11, 2023
Merged

Conversation

HuSharp
Copy link
Contributor

@HuSharp HuSharp commented Jan 10, 2023

Signed-off-by: husharp jinhao.hu@pingcap.com

Signed-off-by: husharp <jinhao.hu@pingcap.com>
Signed-off-by: HuSharp <jinhao.hu@pingcap.com>
Signed-off-by: husharp <jinhao.hu@pingcap.com>
@disksing disksing merged commit de69cb9 into pingcap:master Jan 11, 2023
@ekexium
Copy link
Contributor

ekexium commented Jan 12, 2023

I think changes that break compatibility had better be merged together with dependent PRs. Otherwise it may block other changes

@HuSharp
Copy link
Contributor Author

HuSharp commented Jan 12, 2023

I think changes that break compatibility had better be merged together with dependent PRs. Otherwise it may block other changes

Sry...My fault, should have made a draft label...
I'll do a tikv fix proto firstly

ekexium added a commit to ekexium/kvproto that referenced this pull request Jan 12, 2023
int64 revision = 2;
}

enum ItemKind {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about calling it EventType or something else which can represent that it is an action?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right! from that way we can sync with Etcd

}

message StoreGlobalConfigRequest {
repeated GlobalConfigItem changes = 1;
string config_path = 1;
repeated GlobalConfigItem changes = 2;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

breaking change, shouldn't change the field number

Copy link
Contributor Author

@HuSharp HuSharp Jan 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This interface was previously used by TopSQL as a configuration center control switch, but then chose another way.
tikv/pd/tidb are not use it.
tikv/pd#4308

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants