You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/05/12 13:49:30 UTC
[GitHub] [incubator-doris] BePPPower opened a new pull request, #9533: [refactor] Refactoring Status static methods to format message using …
BePPPower opened a new pull request, #9533:
URL: https://github.com/apache/incubator-doris/pull/9533
…fmt::format
# Proposed changes
Issue Number: close #xxx
## Problem Summary:
1. Almost all of 'Status' static functions have three arguments, but in practice they were used only the first argument, and the second and third arguments use the default values
2. Strings :Substitute is basically used to construct string in the assignment of the first parameter 'msg' (even string splicing method is used in some places), which is tedious and inconsistent to use
## Checklist(Required)
1. Does it affect the original behavior: (No)
3. Has unit tests been added: (No Need)
4. Has document been added or modified: (No Need)
5. Does it need to update dependencies: (No)
6. Are there any changes that cannot be rolled back: (No)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] BePPPower commented on a diff in pull request #9533: [refactor] Refactoring Status static methods to format message using …
Posted by GitBox <gi...@apache.org>.
BePPPower commented on code in PR #9533:
URL: https://github.com/apache/incubator-doris/pull/9533#discussion_r871974393
##########
be/src/agent/agent_server.cpp:
##########
@@ -120,14 +120,14 @@ void AgentServer::submit_tasks(TAgentResult& agent_result,
TTaskType::type task_type = task.task_type;
int64_t signature = task.signature;
-#define HANDLE_TYPE(t_task_type, work_pool, req_member) \
- case t_task_type: \
- if (task.__isset.req_member) { \
- work_pool->submit_task(task); \
- } else { \
- ret_st = Status::InvalidArgument(strings::Substitute( \
- "task(signature=$0) has wrong request member", signature)); \
- } \
+#define HANDLE_TYPE(t_task_type, work_pool, req_member) \
+ case t_task_type: \
+ if (task.__isset.req_member) { \
+ work_pool->submit_task(task); \
+ } else { \
+ ret_st = Status::InvalidArgument("task(signature={}) has wrong request member", \
Review Comment:
ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] yiguolei merged pull request #9533: [refactor] Refactoring Status static methods to format message using …
Posted by GitBox <gi...@apache.org>.
yiguolei merged PR #9533:
URL: https://github.com/apache/doris/pull/9533
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #9533: [refactor] Refactoring Status static methods to format message using …
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9533:
URL: https://github.com/apache/doris/pull/9533#issuecomment-1172879144
PR approved by anyone and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] BePPPower commented on a diff in pull request #9533: [refactor] Refactoring Status static methods to format message using …
Posted by GitBox <gi...@apache.org>.
BePPPower commented on code in PR #9533:
URL: https://github.com/apache/incubator-doris/pull/9533#discussion_r871944860
##########
be/src/agent/agent_server.cpp:
##########
@@ -120,14 +120,14 @@ void AgentServer::submit_tasks(TAgentResult& agent_result,
TTaskType::type task_type = task.task_type;
int64_t signature = task.signature;
-#define HANDLE_TYPE(t_task_type, work_pool, req_member) \
- case t_task_type: \
- if (task.__isset.req_member) { \
- work_pool->submit_task(task); \
- } else { \
- ret_st = Status::InvalidArgument(strings::Substitute( \
- "task(signature=$0) has wrong request member", signature)); \
- } \
+#define HANDLE_TYPE(t_task_type, work_pool, req_member) \
+ case t_task_type: \
+ if (task.__isset.req_member) { \
+ work_pool->submit_task(task); \
+ } else { \
+ ret_st = Status::InvalidArgument("task(signature={}) has wrong request member", \
Review Comment:
Change it like this? : `Status::InvalidArgument("task(signature={}, request menmber={}) has wrong request member", signature, req_menber);`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #9533: [refactor] Refactoring Status static methods to format message using …
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9533:
URL: https://github.com/apache/doris/pull/9533#issuecomment-1172879142
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] cambyzju commented on a diff in pull request #9533: [refactor] Refactoring Status static methods to format message using …
Posted by GitBox <gi...@apache.org>.
cambyzju commented on code in PR #9533:
URL: https://github.com/apache/incubator-doris/pull/9533#discussion_r871939169
##########
be/src/agent/agent_server.cpp:
##########
@@ -120,14 +120,14 @@ void AgentServer::submit_tasks(TAgentResult& agent_result,
TTaskType::type task_type = task.task_type;
int64_t signature = task.signature;
-#define HANDLE_TYPE(t_task_type, work_pool, req_member) \
- case t_task_type: \
- if (task.__isset.req_member) { \
- work_pool->submit_task(task); \
- } else { \
- ret_st = Status::InvalidArgument(strings::Substitute( \
- "task(signature=$0) has wrong request member", signature)); \
- } \
+#define HANDLE_TYPE(t_task_type, work_pool, req_member) \
+ case t_task_type: \
+ if (task.__isset.req_member) { \
+ work_pool->submit_task(task); \
+ } else { \
+ ret_st = Status::InvalidArgument("task(signature={}) has wrong request member", \
Review Comment:
use #req_member to print the real request member, such as 'create_tablet_req', 'drop_tablet_req', and so on, otherwise this error message is still confusing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] cambyzju commented on a diff in pull request #9533: [refactor] Refactoring Status static methods to format message using …
Posted by GitBox <gi...@apache.org>.
cambyzju commented on code in PR #9533:
URL: https://github.com/apache/incubator-doris/pull/9533#discussion_r871949077
##########
be/src/agent/agent_server.cpp:
##########
@@ -120,14 +120,14 @@ void AgentServer::submit_tasks(TAgentResult& agent_result,
TTaskType::type task_type = task.task_type;
int64_t signature = task.signature;
-#define HANDLE_TYPE(t_task_type, work_pool, req_member) \
- case t_task_type: \
- if (task.__isset.req_member) { \
- work_pool->submit_task(task); \
- } else { \
- ret_st = Status::InvalidArgument(strings::Substitute( \
- "task(signature=$0) has wrong request member", signature)); \
- } \
+#define HANDLE_TYPE(t_task_type, work_pool, req_member) \
+ case t_task_type: \
+ if (task.__isset.req_member) { \
+ work_pool->submit_task(task); \
+ } else { \
+ ret_st = Status::InvalidArgument("task(signature={}) has wrong request member", \
Review Comment:
#req_member
like: task(signature={}) missing request member={}
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org