Feature/error handling #2271
Open
Feature/error handling #2271
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
… feature/error_handling_dynamodb
…z-Akshay/aws-athena-query-federation into feature/error-handling
… feature/error_handling_elasticsearch
…ift-snowflake' into feature/error_handling_all
aaronsven
reviewed
Sep 18, 2024
| } | ||
| else if (response.getActiveShards() == 0) { | ||
| throw new RuntimeException("There are no active shards for index (" + index + ")."); | ||
| throw new AthenaConnectorException("There are no active shards for index (" + index + ").", new ErrorDetails().withErrorCode(FederationSourceErrorCode.InternalServiceException.toString())); | ||
| } | ||
| else if (response.getStatus() == ClusterHealthStatus.RED) { |
There was a problem hiding this comment.
AwsRestHighLevelClient only returns the client. the client will be returning a response in ElasticsearchMetadataHandler. so I think that will throw access denied on the metadata handler class as there is not any exclusive exception thrown.
| } | ||
|
|
||
| return domainMap; | ||
| } | ||
| catch (Exception error) { | ||
| throw new RuntimeException("Unable to create domain map: " + error.getMessage(), error); |
There was a problem hiding this comment.
Removed this. As it was present there already so changed only exception.
| @@ -129,13 +132,13 @@ private Map<String, String> getDomainMapFromAmazonElasticsearch() | |||
| } | |||
|
|
|||
| if (domainMap.isEmpty()) { | |||
| throw new RuntimeException("Amazon Elasticsearch Service has no domain information for user."); | |||
| throw new AthenaConnectorException("Amazon Elasticsearch Service has no domain information for user.", new ErrorDetails().withErrorCode(FederationSourceErrorCode.EntityNotFoundException.toString())); | |||
| @@ -153,20 +156,20 @@ private Map<String, String> getDomainMapFromAmazonElasticsearch() | |||
| private Map<String, String> getDomainMapFromEnvironmentVar(String domainMapping) | |||
| { | |||
| if (domainMapping == null || domainMapping.isEmpty()) { | |||
| throw new RuntimeException("Unable to create domain map: Empty or null value found in DomainMapping."); | |||
| throw new AthenaConnectorException("Unable to create domain map: Empty or null value found in DomainMapping.", new ErrorDetails().withErrorCode(FederationSourceErrorCode.EntityNotFoundException.toString())); | |||
| } | ||
| Map<String, String> domainMap; | ||
| try { | ||
| domainMap = domainSplitter.split(domainMapping); | ||
| } | ||
| catch (Exception error) { | ||
| // Intentional obfuscation of error message as it may contain sensitive info (e.g. username/password). | ||
| throw new RuntimeException("Unable to create domain map: DomainMapping Parsing error."); | ||
| throw new AthenaConnectorException("Unable to create domain map: DomainMapping Parsing error.", new ErrorDetails().withErrorCode(FederationSourceErrorCode.InternalServiceException.toString())); |
| } | ||
|
|
||
| if (domainMap.isEmpty()) { | ||
| // Intentional obfuscation of error message: domainMapping contains sensitive info (e.g. username/password). | ||
| throw new RuntimeException("Unable to create domain map: Invalid DomainMapping value."); | ||
| throw new AthenaConnectorException("Unable to create domain map: Invalid DomainMapping value.", new ErrorDetails().withErrorCode(FederationSourceErrorCode.InvalidResponseException.toString())); |
There was a problem hiding this comment.
updated to InvalidInput
|
Can we break this pull request into individual connector and sdk? This is a huge pull request and hard to review |
|
@yipez-spec Below are the PRs raised for individual moduls: I have incorporated the review feedback given by @aaronsven . Please review the above PRs. |
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.
Issue #, if available:
#2256 error handling improvement
Description of changes:
This PR contains error handling changes for connectors with athena-federation-sdk and athena-jdbc :
Snowflake
Redshift
OpenSearch
DynamoDB
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.