Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PLUGIN-1861] Error management for excel plugin #1935

Merged
merged 1 commit into from
Feb 12, 2025

Conversation

psainics
Copy link
Contributor

@psainics psainics commented Feb 11, 2025

Description
Error Management for argument setter exceptions
https://cdap.atlassian.net/browse/PLUGIN-1861

Code change
Modified HTTPArgumentSetter.java
Modified ArgumentSetter.java
Modified pom.xml

Tests
Test Case - tested with Incorrect json value and format

1. Success

Screenshot 2025-02-11 at 1 13 07 PM

2. Invalid Argument JSON format

Screenshot 2025-02-11 at 1 10 52 PM Screenshot 2025-02-11 at 12 49 09 PM Screenshot 2025-02-11 at 12 49 28 PM

3. Missing Argument JSON value

Screenshot 2025-02-11 at 1 09 02 PM Screenshot 2025-02-11 at 1 08 49 PM Screenshot 2025-02-11 at 1 08 30 PM

Comment on lines 54 to 64
return getProgramFailureException((MissingSheetException) t, errorContext,
ErrorType.USER);
}
if (t instanceof ReadException) {
return getProgramFailureException((ReadException) t, errorContext,
ErrorType.USER);
}
if (t instanceof EmptyFileException) {
return getProgramFailureException((EmptyFileException) t, errorContext,
ErrorType.USER);
}
if (t instanceof IllegalArgumentException) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add some subCategories for these errors?

Like for IllegalArgumentException, subCategory can be Configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added !

private static final String ERROR_MESSAGE_FORMAT = "Error occurred in the phase: '%s'. Error message: %s";
private static final String SUBCATEGORY_CONFIGURATION = "Configuration";
private static final String SUBCATEGORY_DATA_MISSING = "Data Missing";
private static final String SUBCATEGORY_FILE_READ_ERROR = "File Read Error";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error sounds redundant since it is already an errorCategory.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.


private static final String ERROR_MESSAGE_FORMAT = "Error occurred in the phase: '%s'. Error message: %s";
private static final String SUBCATEGORY_CONFIGURATION = "Configuration";
private static final String SUBCATEGORY_DATA_MISSING = "Data Missing";
Copy link
Member

@itsankit-google itsankit-google Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Data Integrity sounds more appropriate which can signify that required data is missing or incomplete

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

Copy link
Member

@itsankit-google itsankit-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, please squash commits before merge.

Also cherry-pick the PR in release/2.13

@psainics psainics merged commit 63e92b1 into cdapio:develop Feb 12, 2025
5 checks passed
@psainics psainics deleted the fem/excel branch February 12, 2025 06:53
@psainics psainics restored the fem/excel branch February 12, 2025 07:07
@psainics psainics deleted the fem/excel branch February 16, 2025 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Trigger unit test build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants