Skip to content

5274 strip aws#6837

Merged
kcondon merged 3 commits intoIQSS:developfrom
poikilotherm:5274-strip-aws
Apr 29, 2020
Merged

5274 strip aws#6837
kcondon merged 3 commits intoIQSS:developfrom
poikilotherm:5274-strip-aws

Conversation

@poikilotherm
Copy link
Contributor

What this PR does / why we need it:
@donsizemore asked me to reduce the WAR size file and I wanted to this for a long time anyway.
Now as we move to Payara 5, this is finally possible. This PR reduces the WAR size from 205 MB to 134 MB.

Which issue(s) this PR closes:

Closes #5274

Special notes for your reviewer:
No big changes. Removed completely unused ehcache dependency.
I searched the code base for other AWS SDK (besides S3) usages, but found none.

Suggestions on how to test this:
The whole ZIP file mangling was introduced to make mailing work. This should be tested to ensure it works.
The S3 storage driver should be tested to work, too, as the SDK version moved from 1.11.172 (2017) to 1.11.762 (2020).

Does this PR introduce a user interface change?:
Nope.

Is there a release notes update needed for this change?:
Nope.

Additional documentation:
I'm sure @donsizemore is happy to help testing 😉

Using only the S3 part of AWS SDK pom.xml gets lighter and speeds up builds.
(And hopefully deployment times, too)
@coveralls
Copy link

coveralls commented Apr 17, 2020

Coverage Status

Coverage remained the same at 19.629% when pulling 3ebf690 on poikilotherm:5274-strip-aws into 27ed5dd on IQSS:develop.

@donsizemore
Copy link
Contributor

The integration test suite threw a number of failures on this PR because the PR is intended for Payara 5, and Jenkins is still running Glassfish4 until I get the go-ahead to flip the switch.

I manually ran the same branch through EC2 and everything passed except testUningestFileViaApi, which is being handled in #6821

@pdurbin pdurbin self-assigned this Apr 22, 2020
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

I'm moving this to QA because it seems reasonable. @poikilotherm suggests testing email and S3, which makes sense.

It's flagged as incompatible with Glassfish 4.1 but I'm not entirely sure why. He seems to be matching up Jackson versions with what the app server provides. There's a comment that says, "Should always correspond with Payara bundled version".

The "diff" is relatively small so we have trouble, we should be able to back it out without much difficultly.

@pdurbin pdurbin removed their assignment Apr 22, 2020
@poikilotherm
Copy link
Contributor Author

poikilotherm commented Apr 22, 2020

@pdurbin please see the history of #5274 about the incompatibility with GF 4.1. It was all about Jackson and S3 only working with the Amazon bundled version etc. New apps server, all set. 😎😇

@poikilotherm
Copy link
Contributor Author

I just saw this branch now has conflicts since the merge of #6818
I'll resolve those and push an updated branch.

@kcondon kcondon self-assigned this Apr 23, 2020
@kcondon
Copy link
Contributor

kcondon commented Apr 29, 2020

@poikilotherm This works -S3, zip file. Wasn't sure what you meant when you'd said: "The whole ZIP file mangling was introduced to make mailing work. This should be tested to ensure it works."

@poikilotherm
Copy link
Contributor Author

poikilotherm commented Apr 29, 2020

Hey @kcondon, in the AWS SDK was a properties file for AWS mail services. This was found by scanning the WAR when deploying and broke mail delivery for installations.

That's why that file has been removed from the WAR manually. Mangling happend after packaging with a Maven ZIP plugin, which is now removed from the POM.

So maybe the mail delivery should be tested to be sure it works and no other strange things happen.

@kcondon
Copy link
Contributor

kcondon commented Apr 29, 2020

@poikilotherm OK, mail delivery has been tested and have done general smoke testing as well. I think we're good.

@kcondon kcondon merged commit d73cd26 into IQSS:develop Apr 29, 2020
@djbrooke djbrooke added this to the Dataverse 5 milestone May 1, 2020
@poikilotherm poikilotherm deleted the 5274-strip-aws branch August 10, 2020 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Code Infrastructure formerly "Feature: Code Infrastructure"

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stripping AWS Mail component with TrueZIP blocks packaging for docker images

6 participants