You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@quickstep.apache.org by hbdeshmukh <gi...@git.apache.org> on 2018/04/17 16:19:13 UTC
[GitHub] incubator-quickstep pull request #340: More informative error for BlockNotFo...
GitHub user hbdeshmukh opened a pull request:
https://github.com/apache/incubator-quickstep/pull/340
More informative error for BlockNotFound exception for debugging
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hbdeshmukh/incubator-quickstep informative-storage-error
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-quickstep/pull/340.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #340
----
commit 79de0625c455e661a81437d4620e5d4335d72a0f
Author: Harshad Deshmukh <hb...@...>
Date: 2018-04-17T16:17:58Z
More informative error for BlockNotFound exception for debugging
----
---
[GitHub] incubator-quickstep issue #340: More informative error for BlockNotFound exc...
Posted by jianqiao <gi...@git.apache.org>.
Github user jianqiao commented on the issue:
https://github.com/apache/incubator-quickstep/pull/340
LGTM! Merging.
---
[GitHub] incubator-quickstep issue #340: More informative error for BlockNotFound exc...
Posted by jianqiao <gi...@git.apache.org>.
Github user jianqiao commented on the issue:
https://github.com/apache/incubator-quickstep/pull/340
The change looks good! Some minor fixes are needed.
---
[GitHub] incubator-quickstep pull request #340: More informative error for BlockNotFo...
Posted by jianqiao <gi...@git.apache.org>.
Github user jianqiao commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/340#discussion_r182212888
--- Diff: storage/StorageErrors.hpp ---
@@ -61,9 +61,16 @@ class BlockMemoryTooSmall : public std::exception {
**/
class BlockNotFoundInMemory : public std::exception {
public:
+ BlockNotFoundInMemory(int block_id) : block_id_(block_id) {}
+
virtual const char* what() const throw() {
- return "BlockNotFoundInMemory: The specified block was not found in memory";
+ std::string message = "BlockNotFoundInMemory: The specified block with ID "
+ + std::to_string(block_id_ )+ " was not found in memory";
+ return message.c_str();
}
+
+ private:
+ int block_id_;
--- End diff --
Suggested fix:
```
const std::string block_id_message_;
```
---
[GitHub] incubator-quickstep pull request #340: More informative error for BlockNotFo...
Posted by jianqiao <gi...@git.apache.org>.
Github user jianqiao commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/340#discussion_r182211596
--- Diff: storage/StorageErrors.hpp ---
@@ -61,9 +61,16 @@ class BlockMemoryTooSmall : public std::exception {
**/
class BlockNotFoundInMemory : public std::exception {
public:
+ BlockNotFoundInMemory(int block_id) : block_id_(block_id) {}
+
virtual const char* what() const throw() {
- return "BlockNotFoundInMemory: The specified block was not found in memory";
+ std::string message = "BlockNotFoundInMemory: The specified block with ID "
+ + std::to_string(block_id_ )+ " was not found in memory";
+ return message.c_str();
--- End diff --
The `message` object will be destructed at the end of this method -- so the returned pointer is likely to be invalid.
As a fix we can have the `message` string stored as a member variable.
---
[GitHub] incubator-quickstep pull request #340: More informative error for BlockNotFo...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-quickstep/pull/340
---
[GitHub] incubator-quickstep pull request #340: More informative error for BlockNotFo...
Posted by hbdeshmukh <gi...@git.apache.org>.
Github user hbdeshmukh commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/340#discussion_r182228415
--- Diff: storage/StorageErrors.hpp ---
@@ -61,9 +61,16 @@ class BlockMemoryTooSmall : public std::exception {
**/
class BlockNotFoundInMemory : public std::exception {
public:
+ BlockNotFoundInMemory(int block_id) : block_id_(block_id) {}
+
virtual const char* what() const throw() {
- return "BlockNotFoundInMemory: The specified block was not found in memory";
+ std::string message = "BlockNotFoundInMemory: The specified block with ID "
+ + std::to_string(block_id_ )+ " was not found in memory";
+ return message.c_str();
--- End diff --
My bad, that's a stupid mistake. Thanks for pointing out.
---
[GitHub] incubator-quickstep issue #340: More informative error for BlockNotFound exc...
Posted by hbdeshmukh <gi...@git.apache.org>.
Github user hbdeshmukh commented on the issue:
https://github.com/apache/incubator-quickstep/pull/340
Hi @jianqiao : Made the changes as requested.
---
[GitHub] incubator-quickstep pull request #340: More informative error for BlockNotFo...
Posted by jianqiao <gi...@git.apache.org>.
Github user jianqiao commented on a diff in the pull request:
https://github.com/apache/incubator-quickstep/pull/340#discussion_r182210127
--- Diff: storage/StorageErrors.hpp ---
@@ -61,9 +61,16 @@ class BlockMemoryTooSmall : public std::exception {
**/
class BlockNotFoundInMemory : public std::exception {
public:
+ BlockNotFoundInMemory(int block_id) : block_id_(block_id) {}
--- End diff --
Minor style fix:
```
explicit BlockNotFoundInMemory(const int block_id) : ...
```
---