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

MH-13164, Load catalog for snapshot message effeciently #487

Merged
merged 1 commit into from
Oct 24, 2018

Conversation

lkiesow
Copy link
Member

@lkiesow lkiesow commented Oct 11, 2018

Instead of loading the dublincore XML and de-serialize it into a Java
object only to directly serialize it again as XML, we can just load and
use the XML directly.

@smarquard
Copy link
Contributor

Seems like a good efficiency fix to target at 6.x ?

Instead of loading the dublincore XML and de-serialize it into a Java
object only to directly serialize it again as XML, we can just load and
use the XML directly.
@lkiesow lkiesow force-pushed the t/mh-13164-us-dc-xml-directly branch from f53cefd to 60d324e Compare October 12, 2018 17:58
@lkiesow lkiesow changed the base branch from develop to r/6.x October 12, 2018 17:58
@lkiesow
Copy link
Member Author

lkiesow commented Oct 12, 2018

Rebased to r/6.x

@staubesv staubesv self-requested a review October 18, 2018 11:04
@staubesv staubesv self-assigned this Oct 18, 2018
}
}
return new TakeSnapshot(mp.getIdentifier().compact(), MediaPackageParser.getAsXml(mp), dc,
return new TakeSnapshot(mp.getIdentifier().compact(), MediaPackageParser.getAsXml(mp), dcXml,
Copy link
Contributor

@staubesv staubesv Oct 18, 2018

Choose a reason for hiding this comment

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

@lkiesow The semantic difference is that the former code would have failed early in case of an invalid XML while the new code will not check anything considering that data it loads from the workspace but simply pass the content to TakeSnapshot.

Have you considered possible impacts this could have?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

@staubesv staubesv merged commit 81de4db into opencast:r/6.x Oct 24, 2018
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