RobotError: Fix __repr__()/__str__() issue. #23
Merged
Conversation
This file contains 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
Commit 280f8d3 already tried to improve this, but the reliable way to render an exception message is to pass it as the first argument to the `Exception` superclass constructor; see: https://stackoverflow.com/questions/1319615/proper-way-to-declare-custom-exceptions-in-modern-python This guarantees that both `repr()` and `str()` are set as expected. Until now, for example, `repr()` did not actually render the exception name "RobotError", which was confusing, as all other Python exceptions do that, and the docs say that it should. Similarly, the fix from 280f8d3 actually broke the `str()`, which until now was just `RobotError()`. This commit fixes both these issues.
|
CC @basvandijk |
|
Hey, this problem is back, reintroduced by commit 788b3fd On that commit I get: on the commit just before I get |
aszlig
added a commit
that referenced
this pull request
Jul 25, 2017
Regression introduced by 788b3fd. While setting self.message is documented for setting user-defined exceptions, it might be a misunderstanding by me and it's *not* about the printed result of the exception but about setting custom attributes for an exception. After digging a bit deeper on this, I found that both Python 2.7 and 3.x simply accumulate all arguments passed to __init__() to self.args, see: https://github.com/python/cpython/blob/2.7/Objects/exceptions.c#L65 https://github.com/python/cpython/blob/3.6/Objects/exceptions.c#L66 Testing this with the following example leads to the expected result: class Foo(Exception): def __init__(self): super(Foo, self).__init__(111, "bar") raise Foo() Result for Python 2.7.13: Traceback (most recent call last): File "exctest.py", line 5, in <module> raise Foo() __main__.Foo: (111, 'bar') Result for Python 3.6.1: Traceback (most recent call last): File "exctest.py", line 5, in <module> raise Foo() __main__.Foo: (111, 'bar') So the self.args attribute is just passed to repr() and/or str() and that's about it. Signed-off-by: aszlig <aszlig@redmoonstudios.org> Issue: #23 Cc: @nh2
aszlig
added a commit
to NixOS/nixpkgs
that referenced
this pull request
Dec 4, 2017
New features: * Support for retrieving reverse PTRs. * Support for subnet-ranges. * Add logging (aszlig/hetzner#14). Fixes: * Hide internal methods from the public API. * Fix Python 3 compatibility. * Fix for creating admin accounts with Hetzner's new login site. * Fix __repr__/__str__ issue with some exceptions (aszlig/hetzner#23). * Fix login for RobotWebInterface Changes for the hetznerctl utility: * show: Show subnets * show: Show reverse PTRs * New 'rdns' subcommand for getting/setting/removing reverse-PTRs. * Use 'argparse' instead of 'optparse'. * Add command for managing admin accounts. * New '--debug' flag for printing debugging information. This also fixes NixOS/nixops#778. Tested building against Python 2.7 and Python 3.6. Signed-off-by: aszlig <aszlig@nix.build>
aszlig
added a commit
to NixOS/nixpkgs
that referenced
this pull request
Dec 4, 2017
New features: * Support for retrieving reverse PTRs. * Support for subnet-ranges. * Add logging (aszlig/hetzner#14). Fixes: * Hide internal methods from the public API. * Fix Python 3 compatibility. * Fix for creating admin accounts with Hetzner's new login site. * Fix __repr__/__str__ issue with some exceptions (aszlig/hetzner#23). * Fix login for RobotWebInterface Changes for the hetznerctl utility: * show: Show subnets * show: Show reverse PTRs * New 'rdns' subcommand for getting/setting/removing reverse-PTRs. * Use 'argparse' instead of 'optparse'. * Add command for managing admin accounts. * New '--debug' flag for printing debugging information. This also fixes NixOS/nixops#778. Tested building against Python 2.7 and Python 3.6. Signed-off-by: aszlig <aszlig@nix.build> (cherry picked from commit 6841064) Reason: This unbreaks the NixOps Hetzner target, because the admin sub-account couldn't be created on initial deploy.
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.
Commit 280f8d3 already tried to improve this, but the reliable way
to render an exception message is to pass it as the first argument
to the
Exceptionsuperclass constructor; see:https://stackoverflow.com/questions/1319615/proper-way-to-declare-custom-exceptions-in-modern-python
This guarantees that both
repr()andstr()are set as expected.Until now, for example,
repr()did not actually render the exceptionname "RobotError", which was confusing, as all other Python exceptions
do that, and the docs say that it should.
Similarly, the fix from 280f8d3 actually broke the
str(), whichuntil now was just
RobotError().This commit fixes both these issues.