[rpc] refactor and move createException function #29605
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds a wrapper around the existing createException function that allows passing of an error string, instead of a regular C++ exception. This allows us to createExceptions for errors that aren't necessarilu c++ exceptions. This function is used by #29601 and #26336. Differential Revision: [D18439216](https://our.internmc.facebook.com/intern/diff/D18439216/) [ghstack-poisoned]
rohan-varma
added a commit
that referenced
this pull request
Nov 11, 2019
Adds a wrapper around the existing createException function that allows passing of an error string, instead of a regular C++ exception. This allows us to createExceptions for errors that aren't necessarilu c++ exceptions. This function is used by #29601 and #26336. Differential Revision: [D18439216](https://our.internmc.facebook.com/intern/diff/D18439216/) ghstack-source-id: 93682482 Pull Request resolved: #29605
zhaojuanmao
approved these changes
Nov 11, 2019
Contributor
zhaojuanmao
left a comment
zhaojuanmao
left a comment
There was a problem hiding this comment.
lgtm! please land it after tests are green
Adds a wrapper around the existing `createException` function that allows passing of an error string, instead of a regular C++ exception. This allows us to mark futures with exceptions for errors that aren't necessarily c++ exceptions. This function is used by #29601 and #26336. Differential Revision: [D18439216](https://our.internmc.facebook.com/intern/diff/D18439216/) [ghstack-poisoned]
torch/csrc/distributed/rpc/message.h
Outdated
|
|
||
| // create an exception given a message and exception. | ||
| TORCH_API Message | ||
| createException(const Message& request, const std::exception& e); |
Contributor
There was a problem hiding this comment.
nit: Maybe we should call this createExceptionResponse and clearly state in the docs that we're building a response Message with an exception for the provided request Message.
Adds a wrapper around the existing `createException` function that allows passing of an error string, instead of a regular C++ exception. This allows us to mark futures with exceptions for errors that aren't necessarily c++ exceptions. This function is used by #29601 and #26336. Differential Revision: [D18439216](https://our.internmc.facebook.com/intern/diff/D18439216/) [ghstack-poisoned]
Adds a wrapper around the existing `createException` function that allows passing of an error string, instead of a regular C++ exception. This allows us to mark futures with exceptions for errors that aren't necessarily c++ exceptions. This function is used by #29601 and #26336. Differential Revision: [D18439216](https://our.internmc.facebook.com/intern/diff/D18439216/) [ghstack-poisoned]
rohan-varma
added a commit
that referenced
this pull request
Nov 12, 2019
Pull Request resolved: #29605 Adds a wrapper around the existing createException function that allows passing of an error string, instead of a regular C++ exception. This allows us to createExceptions for errors that aren't necessarilu c++ exceptions. This function is used by #29601 and #26336. ghstack-source-id: 93705275 Differential Revision: [D18439216](https://our.internmc.facebook.com/intern/diff/D18439216/)
pietern
reviewed
Nov 12, 2019
Adds a wrapper around the existing `createException` function that allows passing of an error string, instead of a regular C++ exception. This allows us to mark futures with exceptions for errors that aren't necessarily c++ exceptions. This function is used by #29601 and #26336. Differential Revision: [D18439216](https://our.internmc.facebook.com/intern/diff/D18439216/) [ghstack-poisoned]
rohan-varma
added a commit
that referenced
this pull request
Nov 12, 2019
Pull Request resolved: #29605 Adds a wrapper around the existing createException function that allows passing of an error string, instead of a regular C++ exception. This allows us to createExceptions for errors that aren't necessarilu c++ exceptions. This function is used by #29601 and #26336. ghstack-source-id: 93764993 Differential Revision: [D18439216](https://our.internmc.facebook.com/intern/diff/D18439216/)
pritamdamania87
approved these changes
Nov 12, 2019
|
|
||
| // Create a response Message with an exception type for the provided request | ||
| // message. The passed in string will be used as the created message's payload | ||
| TORCH_API Message createExceptionResponse( |
Contributor
There was a problem hiding this comment.
It doesn't look like this method is used directly. Is there a reason we have this method? If not, can we remove it and only have one createExceptionResponse method for now?
Contributor
Author
There was a problem hiding this comment.
I am planning on using them in the 2 PRs I have linked in the description, so I just added it here to remove it from the other 2.
Adds a wrapper around the existing `createException` function that allows passing of an error string, instead of a regular C++ exception. This allows us to mark futures with exceptions for errors that aren't necessarily c++ exceptions. This function is used by #29601 and #26336. Differential Revision: [D18439216](https://our.internmc.facebook.com/intern/diff/D18439216/) [ghstack-poisoned]
rohan-varma
added a commit
that referenced
this pull request
Nov 13, 2019
Pull Request resolved: #29605 Adds a wrapper around the existing createException function that allows passing of an error string, instead of a regular C++ exception. This allows us to createExceptions for errors that aren't necessarilu c++ exceptions. This function is used by #29601 and #26336. ghstack-source-id: 93803072 Differential Revision: [D18439216](https://our.internmc.facebook.com/intern/diff/D18439216/)
Adds a wrapper around the existing `createException` function that allows passing of an error string, instead of a regular C++ exception. This allows us to mark futures with exceptions for errors that aren't necessarily c++ exceptions. This function is used by #29601 and #26336. Differential Revision: [D18439216](https://our.internmc.facebook.com/intern/diff/D18439216/) [ghstack-poisoned]
rohan-varma
added a commit
that referenced
this pull request
Nov 13, 2019
Pull Request resolved: #29605 Adds a wrapper around the existing createException function that allows passing of an error string, instead of a regular C++ exception. This allows us to createExceptions for errors that aren't necessarilu c++ exceptions. This function is used by #29601 and #26336. ghstack-source-id: 93819039 Differential Revision: [D18439216](https://our.internmc.facebook.com/intern/diff/D18439216/)
Contributor
Author
|
The CI failures are unrelated, will proceed with landing this |
Contributor
|
This pull request has been merged in 3fb9bbc. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stack from ghstack:
Adds a wrapper around the existing
createExceptionfunction thatallows passing of an error string, instead of a regular C++ exception. This
allows us to mark futures with exceptions for errors that aren't necessarily c++
exceptions. This function is used by
#29601 and #26336.
Differential Revision: D18439216