search

conveyal / analysis-backend

Server component of Conveyal Analysis
http://conveyal.com/analysis
MIT License
23 stars 12 forks source link

Patches #187

Closed ansoncfit closed 5 years ago

ansoncfit commented 5 years ago
codecov-io commented 5 years ago

Codecov Report

Merging #187 into dev will increase coverage by 0.08%. The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #187      +/-   ##
============================================
+ Coverage     23.97%   24.05%   +0.08%     
  Complexity      103      103              
============================================
  Files            60       60              
  Lines          2311     2303       -8     
  Branches        211      211              
============================================
  Hits            554      554              
+ Misses         1723     1715       -8     
  Partials         34       34
Impacted Files Coverage Δ Complexity Δ
...nveyal/taui/grids/SeamlessCensusGridExtractor.java 0% <0%> (ø) 0 <0> (ø) :arrow_down:
...java/com/conveyal/taui/models/AnalysisRequest.java 0% <0%> (ø) 0 <0> (ø) :arrow_down:

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 9320ea0...36e8fb7. Read the comment docs.

ansoncfit commented 5 years ago

Would having seamless-census use its own S3 client exacerbate https://github.com/conveyal/analysis-backend/issues/161?

abyrd commented 5 years ago

To summarize: the seamless census project contains com/conveyal/data/census/S3SeamlessSource.java which creates an AmazonS3 client object. It looks like this was copied into analysis-backend as part of an effort to reduce the number of separate AWS client instances, and/or to allow setting the AWS region from which to fetch seamless census data. @ansoncfit has since updated seamless-census library to allow this. Furthermore the S3SeamlessSource in the seamless census project catches exceptions which happen in normal program flow, when census tiles are fetched in empty / ocean areas, while the version copied into analysis-backend did not catch these exceptions.

abyrd commented 5 years ago

I don't expect that this change would have an effect on the connection pool problem in #161. The code inside analysis-backend and the original code in seamless-census both create their own S3 client object.