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

Remove compat #149

Merged
merged 11 commits into from
Jul 26, 2020
Merged

Remove compat #149

merged 11 commits into from
Jul 26, 2020

Conversation

longfangsong
Copy link
Member

@longfangsong longfangsong commented Jun 11, 2020

Signed-off-by: longfangsong longfangsong@icloud.com

Upgrade grpcio and remove the compaction code.

This should fix #148.

Note because of some dependency problems, we have to use my own fork of kvproto. And because of cargo #8258, we cannot use 1.43+ version of rust to build the project.
These limitations have been removed after these crates and tools are updated into new version.

Signed-off-by: longfangsong <longfangsong@icloud.com>
Signed-off-by: longfangsong <longfangsong@icloud.com>
Signed-off-by: longfangsong <longfangsong@icloud.com>
Signed-off-by: longfangsong <longfangsong@icloud.com>
Signed-off-by: longfangsong <longfangsong@icloud.com>
Copy link
Collaborator

@nrc nrc left a comment

Choose a reason for hiding this comment

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

This is great, thanks!

@sticnarf PTAL Do you think we can land this now or should wait kvproto to change? I'm not sure when that might happen.

@sticnarf
Copy link
Collaborator

Looks great.

@nrc
If TiKV changes to use futures 0.3, it's natural that kvproto upgrades its dependency. It's nice to have but it's not an urgent work. I don't expect it to happen very soon.

Anyway, instead of using @longfangsong's personal fork, can we add a branch using futures 0.3 to tikv/kvproto before landing this PR?

@longfangsong
Copy link
Member Author

Anyway, instead of using @longfangsong's personal fork, can we add a branch using futures 0.3 to tikv/kvproto before landing this PR?

I prefer this solution too.

@nrc nrc changed the title Remove compact Remove compat Jul 3, 2020
@nrc
Copy link
Collaborator

nrc commented Jul 23, 2020

@longfangsong could you rebase this PR please? Do we have a branch on kvproto we can use?

@longfangsong
Copy link
Member Author

longfangsong commented Jul 23, 2020

Do we have a branch on kvproto we can use?

I have a pr here: kvproto#633. But it hasn't get merged yet.
It seems I cannot create another branch in pingcap/kvproto, it needs write access of the repo.

# Conflicts:
#	Cargo.toml
#	src/kv_client/mod.rs
#	tikv-client-pd/src/cluster.rs
Signed-off-by: longfangsong <longfangsong@icloud.com>
Signed-off-by: longfangsong <longfangsong@icloud.com>
… .travis.yml

Signed-off-by: longfangsong <longfangsong@icloud.com>
Signed-off-by: longfangsong <longfangsong@icloud.com>
@longfangsong
Copy link
Member Author

Now we are using a branch on pingcap/kvproto.
So I think this pr is ready for merge.
@nrc @sticnarf

Copy link
Collaborator

@nrc nrc left a comment

Choose a reason for hiding this comment

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

lgtm, other than the maybe extra file

src/kv_client/mod.rs Outdated Show resolved Hide resolved
Thanks to @nrc

Signed-off-by: longfangsong <longfangsong@icloud.com>
Copy link
Collaborator

@nrc nrc left a comment

Choose a reason for hiding this comment

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

lgtm!

@nrc nrc merged commit 07194c4 into tikv:master Jul 26, 2020
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.

Upgrade gRPC and remove futures compat code
3 participants