HDDS-14514. [STS] Revamp error handling in endpoint to conform to AWS XML #9674
Conversation
fmorg-git
force-pushed
the
HDDS-14514
branch
2 times, most recently
from
3c11715 to
3230f77
Compare
… conform to AWS minor tweaks
| public class OSTSException extends OS3Exception { | ||
| private static final Logger LOG = LoggerFactory.getLogger(OSTSException.class); | ||
| private static final ObjectMapper MAPPER; | ||
| private static final String AWS_FAULT_NS = "http://webservices.amazon.com/AWSFault/2005-15-09"; |
There was a problem hiding this comment.
Can we make AWS_FAULT_NS and STS_NS public, so that other places(tests) can refer to this directly?
There was a problem hiding this comment.
From what I've read and past experience, it might not be a good idea to refer to the constants from the main logic in the tests. For example, suppose someone in future accidentally changes the value of OSTSException.AWS_FAULT_NS to some incorrect value, then if the tests refer to this constant, the tests will still pass, although the code in the main logic is incorrect. Having separate constants in the main logic and the tests gives a little more safety, because someone would have to change it incorrectly in multiple places for the tests to still pass.
| if (version == null || !version.equals("2011-06-15")) { | ||
| throw new OSTSException( | ||
| "InvalidAction", "Could not find operation " + action + " for version " + | ||
| (version == null ? "NO_VERSION_SPECIFIED" : version), BAD_REQUEST.getStatusCode()); |
There was a problem hiding this comment.
Can we add the expected version number to the error message?
|
Thanks @fmorg-git , left two minor comments, overall it looks good to me. |
|
Thanks @fmorg-git for updating the patch. Wait for the CI to pass. |
2 participants
Please describe your PR in detail:
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-14514
How was this patch tested?
unit tests and smoke tests