-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
All Describe command unified get data from metad #489
Conversation
jenkins go |
Unit testing failed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bravo!
auto spaceId = req.get_space_id(); | ||
auto spaceRet = getSpaceId(req.get_space_name()); | ||
if (!spaceRet.ok()) { | ||
resp_.set_code(to(std::move(spaceRet.status()))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use std::move(spaceRet).status()
instead.
BTW. Please change to
to passing by const reference. Then here std::move
is no longer needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! 👍
@@ -316,7 +330,7 @@ TEST(ProcessorTest, CreateEdgeTest) { | |||
ASSERT_EQ(cpp2::ErrorCode::E_EXISTED, resp.code); | |||
} | |||
{ | |||
// create same name edge in diff spaces | |||
// Create same name edge in diff spaces |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Unit testing passed. |
Unit testing passed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done. Thanks for taking care of it.
auto error = [this] (auto &&e) { | ||
LOG(ERROR) << "Exception caught: " << e.what(); | ||
DCHECK(onError_); | ||
onError_(Status::Error("Internal error")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Output the real error message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message is internal, I think user can’t need konw it directly, but need to Unified classification of error information in the future. Please accept it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense.
auto error = [this] (auto &&e) { | ||
LOG(ERROR) << "Exception caught: " << e.what(); | ||
DCHECK(onError_); | ||
onError_(Status::Error("Internal error")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
Unit testing passed. |
Unit testing failed. |
jenkins go |
Unit testing passed. |
close #435 |
* Describe Tag and Edge from metad * Address dutor's comment * Fix to
* Describe Tag and Edge from metad * Address dutor's comment * Fix to
1、Describe Tag and Edge from metad
2、"Use space" get spaceId from metad
3、Modify DescribeSpace