fix: TOOLS-3012 standardise error handling string for manage roster #337
Conversation
a-spiker
commented
Feb 20, 2025
•
a-spiker
commented
- removes special string handling
- removes special string handling
a-spiker
changed the title
| @@ -2067,7 +2067,7 @@ async def test_info_cluster_stable_with_errors(self): | |||
| ) | |||
|
|
|||
| self.info_mock.return_value = "ERROR::unstable cluster" | |||
| expected = ASInfoClusterStableError("ERROR::unstable cluster") | |||
| expected = ASInfoResponseError("Server returned an error response for info command", "ERROR::unstable cluster") | |||
There was a problem hiding this comment.
This looks like it would still fail since it would be ERROR:7:unstable cluster
There was a problem hiding this comment.
@kportertx We have removed special string handling with this PR and treated every error generically. With this change, we always handle the Exception, which goes to a happy path where the User can choose what to do next.
Without this change, we were abstracting the Error thrown from the server. Here's a complete experience example.
Admin+> manage roster stage observed ns sc
ERROR: Server returned an error response for info command : unstable cluster.
WARNING: It is advised that you do not manage the roster. Run 'info network' for more information.
You are about to set the pending-roster for namespace sc to: A2, A1, A0
Confirm that you want to proceed by typing x5d8ee, or cancel by typing anything else.
x5d8ee
Pending roster now contains observed nodes.
Run "manage recluster" for your changes to take effect.
There was a problem hiding this comment.
This path was broken due to unstable-cluster -> unstable cluster change from the server where asadm was handling these special strings viz unstable cluster, cluster not specified size
There was a problem hiding this comment.
K, just looked like this test would still fail.
| @@ -2053,7 +2053,7 @@ async def test_info_cluster_stable(self): | |||
|
|
|||
| async def test_info_cluster_stable_with_errors(self): | |||
| self.info_mock.return_value = "ERROR::cluster not specified size" | |||
| expected = ASInfoClusterStableError("ERROR::cluster not specified size") | |||
| expected = ASInfoResponseError("Server returned an error response for info command", "ERROR::cluster not specified size") | |||
There was a problem hiding this comment.
Can we directly use INFO_SERVER_ERROR_RESPONSE here rather than the hardcoded string?
