Refactor AWS client creation

[evansiroky]

Checklist

• Appropriate branch selected (all PRs must first be merged to dev before they can be merged to master)
• Any modified or new methods or classes have helpful JavaDoc and code is thoroughly commented <---- working on it
• The description lists all applicable issues this PR seeks to resolve
• The description lists any configuration setting(s) that differ from the default settings
• All tests and CI builds passing

Description

The creation of various AWS clients was happening in various random places and in multiple different methods of creation. This PR seeks to standardize the creation of AWS clients in a central place in the AWSUtils class. All classes that use an AWS client have been refactored to obtain a client from the AWSUtils class. Within the AWSUtils class, each AWS client is created and stored in an in-memory cache. Furthermore, clients that are dependent on a role (and thus the creation of a session) will automatically renew their session/client as needed.

This PR also adds additional exception handling in various places as this refactor will throw non-runtime exceptions which the compiler will flag as needed to be caught. This can help identify places where AWS exceptions that are Runtime Exceptions (which the compiler doesn't flag as needing to be caught) are probably thrown.

This PR is built on top of #331.

[codecov-commenter]

Codecov Report

Merging #332 into dev will decrease coverage by 0.42%.
The diff coverage is 0.29%.

@@             Coverage Diff              @@
##                dev     #332      +/-   ##
============================================
- Coverage     24.04%   23.62%   -0.43%     
+ Complexity      542      538       -4     
============================================
  Files           169      172       +3     
  Lines          8895     8996     +101     
  Branches       1230     1223       -7     
============================================
- Hits           2139     2125      -14     
- Misses         6543     6661     +118     
+ Partials        213      210       -3     

Flag	Coverage Δ	Complexity Δ
#unit_tests	23.62% <0.29%> (-0.43%)	538.00 <0.00> (-4.00)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ	Complexity Δ
...veyal/datatools/common/utils/AWSClientManager.java	0.00% <0.00%> (ø)	0.00 <0.00> (?)
.../com/conveyal/datatools/common/utils/AWSUtils.java	0.00% <0.00%> (-7.70%)	0.00 <0.00> (-4.00)
...al/datatools/common/utils/CheckedAWSException.java	0.00% <0.00%> (ø)	0.00 <0.00> (?)
...conveyal/datatools/common/utils/ExpiringAsset.java	0.00% <0.00%> (ø)	0.00 <0.00> (?)
...ols/editor/controllers/api/SnapshotController.java	15.21% <0.00%> (+0.48%)	2.00 <0.00> (ø)
...datatools/editor/jobs/ExportSnapshotToGTFSJob.java	36.95% <0.00%> (-3.52%)	4.00 <0.00> (ø)
.../manager/controllers/api/DeploymentController.java	8.63% <0.00%> (-0.26%)	2.00 <0.00> (ø)
...manager/controllers/api/FeedVersionController.java	12.33% <0.00%> (ø)	2.00 <0.00> (ø)
...ols/manager/controllers/api/ProjectController.java	12.58% <0.00%> (ø)	2.00 <0.00> (ø)
...ools/manager/controllers/api/ServerController.java	3.13% <0.00%> (-0.63%)	1.00 <0.00> (ø)
... and 14 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 732e344...ba18cc2. Read the comment docs.

[evansiroky]

I'm calling the request to fix #269 out of scope for this PR.

Also, I ended up adding some more fault tolerance to the DeployJob so that it will properly fail in certain circumstances and still gracefully wait for the termination of the RecreateBuildImageJob.

[binh-dam-ibigroup]

This looks great. Looks like @landonreed swept through everything already. I have a few minor questions/suggestions as noted but otherwise this can be merged.

[binh-dam-ibigroup]

The previous syntax looks cleaner. I am not sure I like this pattern of declaring a variable then assigning it inside of the try/catch block, although we may be stuck with that.

[binh-dam-ibigroup]

How would a syntax like the one below work?

CreateImageResult createImageResult = tryCreateImage(...);
if (createImageResult) {
   // do the things.
}

[evansiroky]

There would still need to be a clause to handle the failure case, so I think this actually adds more code.

[landonreed]

@evansiroky, looks to be good to merge. I'm going to apply Blocked and assign to you to do some further testing with MTC-QA instance and for deploying to ibi-dev. Once that's done, we can move forward with merging.

[evansiroky]

Some changes have been pushed to address PR review, and this has been verified to work for MTC-QA. I'm removing the blocked tag and reassigned for review.

[landonreed]

🎉 This PR is included in version 3.8.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀