Less aws requests #343

evansiroky · 2020-10-03T05:15:28Z

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
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

This PR is built on top of #341.

This PR does a few things to reduce the amount of AWS requests that are performed as follows:

Don't include ec2 instance data with OtpServers in API calls to get a Project or Projects
Don't include ec2 instance data with OtpServers in API calls to get all servers
Add new endpoint for getting a single OtpServer
During MonitorServerStatusJobs only check instance health after waiting for 4 seconds instead of both before and after

This PR has a companion PR for the UI here: ibi-group/datatools-ui#615

codecov-commenter · 2020-10-03T05:18:53Z

Codecov Report

Merging #343 into dev will decrease coverage by 0.07%.
The diff coverage is 2.60%.

@@             Coverage Diff              @@
##                dev     #343      +/-   ##
============================================
- Coverage     23.72%   23.65%   -0.08%     
- Complexity      545      547       +2     
============================================
  Files           172      175       +3     
  Lines          8998     9038      +40     
  Branches       1225     1226       +1     
============================================
+ Hits           2135     2138       +3     
- Misses         6654     6692      +38     
+ Partials        209      208       -1

Flag	Coverage Δ	Complexity Δ
#unit_tests	`23.65% <2.60%> (-0.08%)`	`547.00 <2.00> (+2.00)`	⬇️

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

Impacted Files	Coverage Δ	Complexity Δ
...conveyal/datatools/common/utils/ExpiringAsset.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
...l/datatools/common/utils/aws/AWSClientManager.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...atatools/common/utils/aws/CheckedAWSException.java	`0.00% <ø> (ø)`	`0.00 <0.00> (?)`
.../conveyal/datatools/common/utils/aws/EC2Utils.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...atatools/common/utils/aws/EC2ValidationResult.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
.../conveyal/datatools/common/utils/aws/IAMUtils.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...m/conveyal/datatools/common/utils/aws/S3Utils.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...tools/editor/controllers/api/EditorController.java	`33.20% <0.00%> (-3.86%)`	`18.00 <0.00> (ø)`
...ols/editor/controllers/api/SnapshotController.java	`15.21% <0.00%> (ø)`	`2.00 <0.00> (ø)`
...datatools/editor/jobs/ExportSnapshotToGTFSJob.java	`36.95% <0.00%> (ø)`	`4.00 <0.00> (ø)`
... and 23 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 3cab4b4...3b5d056. Read the comment docs.

landonreed · 2020-10-05T20:18:32Z

src/main/java/com/conveyal/datatools/manager/models/Project.java

+     * Usually a mixin would be used on an external class, but since we are changing one thing about a single class, it
+     * seemed unnecessary to define a new view.
+     */
+    public abstract static class ProjectWithoutOtpServers {


I don't understand why this is needed. Isn't otpServers omitted by default? The Mixin above is only needed to add the otpServers property. Also, the comment is just copy pasta.

Ah, you're right. Not sure why I added it.

landonreed · 2020-10-05T20:19:42Z

src/main/java/com/conveyal/datatools/manager/models/OtpServer.java

+
+    /**
+     * A MixIn to be applied to this OtpServer, for returning a single OtpServer, without EC2InstanceSummaries that
+     * don't need to be written in JSON output.


This comment is inverted. The mixin is used to return a list of OtpServers.

landonreed

Comments need some cleanup and I'm not sure that ProjectWithoutOtpServers needs to exist.

evansiroky · 2020-10-06T19:25:02Z

I made the requested changes.

codecov-io · 2020-10-06T19:33:42Z

Codecov Report

Merging #343 into dev will increase coverage by 0.00%.
The diff coverage is 50.00%.

@@            Coverage Diff            @@
##                dev     #343   +/-   ##
=========================================
  Coverage     24.01%   24.02%           
- Complexity      560      561    +1     
=========================================
  Files           175      175           
  Lines          9055     9068   +13     
  Branches       1228     1231    +3     
=========================================
+ Hits           2175     2179    +4     
- Misses         6672     6681    +9     
  Partials        208      208

Flag	Coverage Δ	Complexity Δ
#unit_tests	`24.02% <50.00%> (+<0.01%)`	`561.00 <1.00> (+1.00)`

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

Impacted Files	Coverage Δ	Complexity Δ
...datatools/manager/jobs/MonitorServerStatusJob.java	`0.00% <ø> (ø)`	`0.00 <0.00> (ø)`
...m/conveyal/datatools/manager/models/OtpServer.java	`6.66% <0.00%> (-0.23%)`	`2.00 <0.00> (ø)`
...com/conveyal/datatools/manager/models/Project.java	`66.66% <ø> (ø)`	`6.00 <0.00> (ø)`
...ools/manager/controllers/api/ServerController.java	`11.32% <50.00%> (+1.74%)`	`2.00 <1.00> (+1.00)`
...ols/manager/controllers/api/ProjectController.java	`13.19% <100.00%> (+0.60%)`	`2.00 <0.00> (ø)`

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 5f6e68e...0f7677a. Read the comment docs.

binh-dam-ibigroup

The incremental changes from #341 to #343 look fine to me.

landonreed · 2020-10-30T17:27:59Z

🎉 This PR is included in version 3.8.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

evansiroky added 2 commits October 2, 2020 16:37

refactor: reduce AWS requests in various ways

016500d

refactor: remove unused import

3b5d056

evansiroky added the BLOCKED label Oct 3, 2020

evansiroky assigned landonreed and binh-dam-ibigroup Oct 3, 2020

evansiroky mentioned this pull request Oct 3, 2020

Lazy-load server data with ec2 instance info ibi-group/datatools-ui#615

Merged

5 tasks

landonreed reviewed Oct 5, 2020

View reviewed changes

landonreed suggested changes Oct 5, 2020

View reviewed changes

landonreed assigned evansiroky and unassigned landonreed Oct 5, 2020

refactor: improve comments and remove unneeded code

b2b0b9b

evansiroky assigned landonreed and unassigned evansiroky Oct 6, 2020

binh-dam-ibigroup approved these changes Oct 6, 2020

View reviewed changes

binh-dam-ibigroup removed their assignment Oct 6, 2020

landonreed approved these changes Oct 7, 2020

View reviewed changes

landonreed assigned evansiroky and unassigned landonreed Oct 7, 2020

Merge branch 'dev' into less-aws-requests

0f7677a

evansiroky merged commit 6e23715 into dev Oct 15, 2020

landonreed mentioned this pull request Oct 30, 2020

Release #352

Merged

8 tasks

landonreed added the released label Oct 30, 2020

binh-dam-ibigroup deleted the less-aws-requests branch July 10, 2023 21:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Less aws requests #343

Less aws requests #343

evansiroky commented Oct 3, 2020 •

edited

Loading

codecov-commenter commented Oct 3, 2020 •

edited

Loading

landonreed Oct 5, 2020 •

edited

Loading

evansiroky Oct 6, 2020

landonreed Oct 5, 2020 •

edited

Loading

landonreed left a comment

evansiroky commented Oct 6, 2020

codecov-io commented Oct 6, 2020 •

edited

Loading

binh-dam-ibigroup left a comment

landonreed commented Oct 30, 2020

Less aws requests #343

Less aws requests #343

Conversation

evansiroky commented Oct 3, 2020 • edited Loading

Checklist

Description

codecov-commenter commented Oct 3, 2020 • edited Loading

Codecov Report

landonreed Oct 5, 2020 • edited Loading

Choose a reason for hiding this comment

evansiroky Oct 6, 2020

Choose a reason for hiding this comment

landonreed Oct 5, 2020 • edited Loading

Choose a reason for hiding this comment

landonreed left a comment

Choose a reason for hiding this comment

evansiroky commented Oct 6, 2020

codecov-io commented Oct 6, 2020 • edited Loading

Codecov Report

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

landonreed commented Oct 30, 2020

evansiroky commented Oct 3, 2020 •

edited

Loading

codecov-commenter commented Oct 3, 2020 •

edited

Loading

landonreed Oct 5, 2020 •

edited

Loading

landonreed Oct 5, 2020 •

edited

Loading

codecov-io commented Oct 6, 2020 •

edited

Loading