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

server: add Debug service #2299

Merged
merged 22 commits into from Sep 20, 2017

Conversation

Projects
None yet
5 participants
@overvenus
Member

overvenus commented Sep 15, 2017

Add debug framework, and impl get rpc.

@overvenus overvenus requested review from siddontang, nolouch and hicqu Sep 15, 2017

@overvenus overvenus changed the title from [WIP] server: add Debug service to server: add Debug service Sep 15, 2017

@overvenus

This comment has been minimized.

Show comment
Hide comment
@overvenus

overvenus Sep 15, 2017

Member

PTAL

Member

overvenus commented Sep 15, 2017

PTAL

overvenus added some commits Sep 17, 2017

@siddontang

This comment has been minimized.

Show comment
Hide comment
@siddontang

siddontang Sep 18, 2017

Contributor

PTAL @BusyJay

Contributor

siddontang commented Sep 18, 2017

PTAL @BusyJay

@overvenus

This comment has been minimized.

Show comment
Hide comment
@overvenus

overvenus Sep 18, 2017

Member

/run-all-test

Member

overvenus commented Sep 18, 2017

/run-all-test

@overvenus

This comment has been minimized.

Show comment
Hide comment
@overvenus

overvenus Sep 19, 2017

Member

PTAL

Member

overvenus commented Sep 19, 2017

PTAL

@overvenus

This comment has been minimized.

Show comment
Hide comment
@overvenus

overvenus Sep 19, 2017

Member

/run-all-test

Member

overvenus commented Sep 19, 2017

/run-all-test

@siddontang

This comment has been minimized.

Show comment
Hide comment
@siddontang
Contributor

siddontang commented Sep 19, 2017

Show outdated Hide outdated src/server/server.rs
Show outdated Hide outdated src/server/server.rs
Show outdated Hide outdated src/server/service/debug.rs

overvenus added some commits Sep 19, 2017

@hicqu

This comment has been minimized.

Show comment
Hide comment
@hicqu

hicqu Sep 19, 2017

Contributor

LGTM.

Contributor

hicqu commented Sep 19, 2017

LGTM.

let f = resp.then(|v| match v {
Ok(resp) => sink.success(resp).map_err(on_error),
Err(Error::NotFound(msg)) => {
let status = RpcStatus::new(RpcStatusCode::NotFound, Some(msg));

This comment has been minimized.

@BusyJay

BusyJay Sep 19, 2017

Contributor

Any tests to cover it?

@BusyJay

BusyJay Sep 19, 2017

Contributor

Any tests to cover it?

Show outdated Hide outdated src/server/server.rs
Show outdated Hide outdated src/raftstore/store/debug.rs
quick_error!{
#[derive(Debug)]
pub enum Error {
InvalidArgument(msg: String) {

This comment has been minimized.

@AndreMouche

AndreMouche Sep 19, 2017

Member

Could we use &'static str instead of String?

@AndreMouche

AndreMouche Sep 19, 2017

Member

Could we use &'static str instead of String?

This comment has been minimized.

@overvenus

overvenus Sep 19, 2017

Member

I think String is more flexible and Debug service is not performance-critical.

@overvenus

overvenus Sep 19, 2017

Member

I think String is more flexible and Debug service is not performance-critical.

Show outdated Hide outdated tests/raftstore/test_service.rs
Show outdated Hide outdated src/raftstore/store/debug.rs
Show outdated Hide outdated src/raftstore/store/debug.rs
Show outdated Hide outdated src/server/service/debug.rs
Show outdated Hide outdated src/raftstore/store/debug.rs
Show outdated Hide outdated src/raftstore/store/debug.rs
@BusyJay

This comment has been minimized.

Show comment
Hide comment
@BusyJay

BusyJay Sep 20, 2017

Contributor

LGTM

Contributor

BusyJay commented Sep 20, 2017

LGTM

@tikv tikv deleted a comment from hicqu Sep 20, 2017

@hicqu

This comment has been minimized.

Show comment
Hide comment
@hicqu

hicqu Sep 20, 2017

Contributor

LGTM.

Contributor

hicqu commented Sep 20, 2017

LGTM.

@hicqu

hicqu approved these changes Sep 20, 2017

@overvenus

This comment has been minimized.

Show comment
Hide comment
@overvenus

overvenus Sep 20, 2017

Member

/run-all-test

Member

overvenus commented Sep 20, 2017

/run-all-test

@overvenus overvenus merged commit efb705e into master Sep 20, 2017

5 of 6 checks passed

jenkins-ci-tikv/unit-test Jenkins job failed
Details
ci/circleci Your tests passed on CircleCI!
Details
jenkins-ci-tikv/build Jenkins job succeeded.
Details
jenkins-ci-tikv/integration-common-test Jenkins job succeeded.
Details
jenkins-ci-tikv/integration-ddl-test Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@overvenus overvenus deleted the ov/debugger branch Sep 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment