numo-labs / aws-lambda-helper

:lollipop: Collection of helper methods for lambda
GNU General Public License v3.0
22 stars 2 forks source link

Bugfix s3 error issue 59 #60

Closed nelsonic closed 8 years ago

nelsonic commented 8 years ago

@jruts this PR should fix both:

Note: it's a backward-compatible change but I've increased the minor version number because it has changed functionality...

lennym commented 8 years ago

One possible issue with switching the order of operations of the s3 write and publish of results is that the s3 write will potentially be performing a number of save operations, and waits until all are complete before passing on any results.

We might want to do something a little like here and here where the results are passed on incrementally as the per-result iterator completes to avoid waiting for all s3 save operations to complete before sending the first result.

codecov-io commented 8 years ago

Current coverage is 100%

Merging #60 into master will not change coverage

@@             master        #60   diff @@
==========================================
  Files             1          1          
  Lines           157        161     +4   
  Methods          25         26     +1   
  Messages          0          0          
  Branches         31         32     +1   
==========================================
+ Hits            157        161     +4   
  Misses            0          0          
  Partials          0          0          

Sunburst

Powered by Codecov. Last updated by a32b6ca...33ee7c4

nelsonic commented 8 years ago

@lennym does the fact that we're using the ee in the lambda not already cover us for that? i.e. the results are already being individually pushed by the lambda so we should only be saving one record at a time...?

lennym commented 8 years ago

@nelsonic For packages we get them one at a time, but for tiles we still get them en masse.

lennym commented 8 years ago

I think.

lennym commented 8 years ago

It probably won't be an issue, but it's worth keeping an eye on if we see a performance hit.

nelsonic commented 8 years ago

@lennym agree. I think @tomgco is implementing monitoring. 👍