Skip to content

Adding an aggregate exception class to the interface module to be use… - #436

Merged
GraciesPadre merged 2 commits into
SpectraLogic:3_2_11from
rpmoore:3_2_11
Feb 2, 2017
Merged

Adding an aggregate exception class to the interface module to be use…#436
GraciesPadre merged 2 commits into
SpectraLogic:3_2_11from
rpmoore:3_2_11

Conversation

@rpmoore

@rpmoore rpmoore commented Feb 2, 2017

Copy link
Copy Markdown
Contributor

…d to return back multiple exceptions from the metadata restoration logic.

…d to return back multiple exceptions from the metadata restoration logic.
@@ -45,17 +54,41 @@ public void metadataReceived(final String filename, final Metadata metadata) {
* @param metadata metadata which needs to be set on local file
*/
private void restoreMetaData(final String objectName, final Metadata metadata) throws IOException, InterruptedException {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should this method throw AggregateException now?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's not required since I made AggregateException extend RuntimeException.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@shabtaisharon

Copy link
Copy Markdown
Contributor

you don't want to add test for the new AggregateException?

@rpmoore

rpmoore commented Feb 2, 2017

Copy link
Copy Markdown
Contributor Author

I probably should...

this.exceptions = exceptionList(exceptions);
}

private static List<Throwable> exceptionList(final Iterable<Throwable> exceptions) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure I understand the need to make a mutable copy of the iterable contents, since the getter returns an iterable. Though it's most likely not a big deal in terms of the amount of space used to make a copy, I'm wondering about the immutable -> mutable ness.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The main reason is I wanted to make sure that the iterable we return in the getter can be called multiple times. I wanted to make the list immutable, but we've not included guava in the interfaces module, which is why I made it with a mutable type.

@GraciesPadre
GraciesPadre merged commit 9408b66 into SpectraLogic:3_2_11 Feb 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants