search

numo-labs / aws-lambda-helper

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

Remove items with no url prop when saving to S3 on publish #57

Closed lennym closed 8 years ago

lennym commented 8 years ago

We want to be able to send items back into the results pipe to the client without also necessarily saving them to s3 as well. In particular, it does not make sense to save items to s3 which do not url.

As such, we remove items from the items array which have no url property before saving to s3.

Also slightly refactor the tests for this method to more explicitly define the calls to downstream AWS services by stubbing the AWS SDK.

codecov-io commented 8 years ago

Current coverage is 100%

Merging #57 into master will not change coverage

@@             master        #57   diff @@
==========================================
  Files             1          1          
  Lines           153        156     +3   
  Methods          25         25          
  Messages          0          0          
  Branches         29         30     +1   
==========================================
+ Hits            153        156     +3   
  Misses            0          0          
  Partials          0          0

Sunburst

Powered by Codecov. Last updated by 0ff31a4...12fa1a1

lennym commented 8 years ago

On further thought, we might want to be more explicit and filter items based on the type property. Since we're really saving packages and articles for reference later, so a type: 'package' or type: 'tile' filter might be more appropriate that a slightly arbitrary url filter..

nelsonic commented 8 years ago

@lennym that would require us to add a type property to each item that we want to store on S3. I don't have an objection to doing that but it is more work (in 3 places), whereas your url filter does the job already.

lennym commented 8 years ago

@nelsonic I think those items have those type properties already as far as I can tell. Happy to merge as is though if you are.

lennym commented 8 years ago

I stand corrected. Tiles and filters have a type property, packages do not.

nelsonic commented 8 years ago

@lennym you're right, lambda-tile-provider/saveHandler.js#L38 does specify its type and we can easily add it to the packages... Let me write a quick PR to update lambda-ne-classic-package-provider to add type='package' while you update this PR to check for that type of content. 👍

nelsonic commented 8 years ago

@lennym please take a look at: https://github.com/numo-labs/lambda-ne-classic-package-provider/pull/76 does this allow us to perform the more explicit check now in saveRecord ?

lennym commented 8 years ago

@nelsonic Updated to filter based on type rather than url.

nelsonic commented 8 years ago

@jruts what type of record are you planning on assigning for the records stored by lambda-search-request-handler for the neo4j response?

Context: aws-lambda-helper save to S3 method will now check for specific types of content before saving see code in this PR.

nelsonic commented 8 years ago

looks good. let's merge! 👍