You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "infdahai (via GitHub)" <gi...@apache.org> on 2023/05/03 06:50:43 UTC
[GitHub] [incubator-kvrocks] infdahai opened a new issue, #1416: common/ParseUtil replace erro msgs with error integers
infdahai opened a new issue, #1416:
URL: https://github.com/apache/incubator-kvrocks/issues/1416
### Search before asking
- [X] I had searched in the [issues](https://github.com/apache/incubator-kvrocks/issues) and found no similar issues.
### Motivation
for exmaple, one instantiation of ParseInt presents below.
https://github.com/apache/incubator-kvrocks/blob/30e9fd788a4acb585566a008f4d5fff65ee06ce3/src/common/parse_util.h#L113-L125
We can't get a more detailed information about what `v` meanings in Parse() func and just think it as an non-integer error if the parsing occurs the error.
I view the source code about calling `ParseInt`(or `ParseType`) functions and find that we use the parse function without taking advantage of the info from error msgs.
```cpp
auto parse_result = ParseInt<int64_t>(args[3], 10);
if (!parse_result) {
return {Status::RedisParseErr, "Invalid version"};
}
```
We can provide an exact error msg actually, such as `out of range about the version`, `illegal character of the 'version' str`.
This is a issue for helpful msgs, so it is not necessary.
### Solution
It needs a lot of work here.
we just defines different error types in parse_util module and the concrete caller use the type to modify error-msgs.
### Are you willing to submit a PR?
- [ ] I'm willing to submit a PR!
--
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: issues-unsubscribe@kvrocks.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on issue #1416: common/ParseUtil replace erro msgs with error integers
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on issue #1416:
URL: https://github.com/apache/incubator-kvrocks/issues/1416#issuecomment-1532544345
I think it seems not necessary to create status codes for every parsing error type. In kvrocks, a better and sample solution already exists:
```
auto version = ParseInt<VersionInteger>(version_str);
if (!version) {
return version.Prefixed("failed to parse version");
}
```
Then users will get a well-formed error message, like `failed to parse version: out of range` or `failed to parse version: encounter non-digit characters`.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] infdahai closed issue #1416: common/ParseUtil replace erro msgs with error integers
Posted by "infdahai (via GitHub)" <gi...@apache.org>.
infdahai closed issue #1416: common/ParseUtil replace erro msgs with error integers
URL: https://github.com/apache/incubator-kvrocks/issues/1416
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] infdahai commented on issue #1416: common/ParseUtil replace erro msgs with error integers
Posted by "infdahai (via GitHub)" <gi...@apache.org>.
infdahai commented on issue #1416:
URL: https://github.com/apache/incubator-kvrocks/issues/1416#issuecomment-1532581231
yes, `v.Prefixed("failed to parse version");` is enough.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] infdahai closed issue #1416: common/ParseUtil replace erro msgs with error integers
Posted by "infdahai (via GitHub)" <gi...@apache.org>.
infdahai closed issue #1416: common/ParseUtil replace erro msgs with error integers
URL: https://github.com/apache/incubator-kvrocks/issues/1416
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org