@dubeejw can we close this PR? You can setup travis on your own repository if you want to have full travis coverage.

@rabbah, @markusthoemmes, a cache concurrency test fails on this one. I have figured out the failure is due the need of two DB writes for saving attachments. Where the first write is for the DB document and the second write is for the DB document attachment.

When only caching Java actions as done in #2855 and implementing the same concurrency test for Java actions, the test will fail as well.

Failure output shown here. The test fails because update does two writes as stated above, so a concurrent get request can fail as update does not finish as quickly for attachments.

whisk.core.database.test.CacheConcurrencyTests > the cache should support concurrent CRUD without bogus residual cache entries, iter 1 FAILED
    org.scalatest.exceptions.TestFailedException: update+get: failed for testy4 404 was not equal to 200

Any suggestions?

I did some hacking to only do a put on the DB document if it did not already exist. This allowed the test above to pass, but does not account for the document itself being updated. For instance, if a memory limit is updated from a REST call the document wouldn't be updated.

I suppose another option would be to change the test to accept a 200 or 404 from the get requests.

Codecov Report

Merging #3286 into master will decrease coverage by 0.13%.
The diff coverage is 90%.

@@            Coverage Diff             @@
##           master    #3286      +/-   ##
==========================================
- Coverage   74.59%   74.45%   -0.14%     
==========================================
  Files         126      126              
  Lines        5982     5961      -21     
  Branches      397      384      -13     
==========================================
- Hits         4462     4438      -24     
- Misses       1520     1523       +3

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 6a3d6bd...b415146. Read the comment docs.

Closing #2847 in favor of this PR as it was easier to rebase with the merged caching changes.

@chetanmeh, do you think these changes will be useful in the future?

@dubee Yes this would be useful as document size limitation is impacting nodejs kind also for us. So would like to convert all types to attachments. With support for Inlined attachments #3709 switching to attachment should not have negative impact on small functions.

@dubee Do you plan to work on this otherwise I can look into it. Given recent changes in master the ArtifactStore does not make use of attachmentName attribute from manifest. So was thinking to do away with manifest entry for name attribute.

For attachment type I am not sure - For e.g. if its a simple code snippet then type can be text/plain otherwise if its a zip (say js code packaged as zip) then it would be application/octet-stream.

Further with inlining support now we can always store the code for all kinds as attachments by design and let ArtifactStore inline the binary if its small. So no need to differentiate between attachment for various kinds.

Any such change should be done in a backward compatible way to ensure existing actions work as expected

@dubee Ping

@dubee any news on this one?

PR #3945 supersedes this one