search

dawson-org / dawson-cli

A serverless web framework for Node.js on AWS (CloudFormation, CloudFront, API Gateway, Lambda)
https://dawson.sh
GNU General Public License v3.0
713 stars 25 forks source link

python-support #136

Closed lusentis closed 6 years ago

lusentis commented 7 years ago

Example app: https://github.com/dawson-org/dawson-examples/tree/master/2-basic-python

lusentis commented 7 years ago

@alexcasalboni tests are passing on this branch, if you can, rebase your fork :)

alexcasalboni commented 7 years ago

@lusentis will do! I also wanted to improve tests and make sure the JSON printed to stdout is always valid, so that you can proceed with the whole pipeline in JS without too much hacking. Then I'll need to work on the Python wrapper too.

Would you prefer making createIndex.getWrappingCode a bit more generic to handle every possible case, or creating a new createIndex.getWrappingPythonCode function? How will we choose which one to call? The language/runtime choice should be available along the pipeline.

lusentis commented 7 years ago

@alexcasalboni Great!

I'd propose refactoring the current implementation like this:

In a near future we could also support mixed-language projects? Does this makes sense?

What do you think? (I'll look into this deeper later this afternoon.)

lusentis commented 7 years ago

Okay, I've ended up with a slightly different and more clean approach. I've moved createIndex to language-javascript-latest and kept the whole language choice logic in createBundle.js (and config.js), so it will be much easier to add more languages in the future.

Feedback is appreciated :D

alexcasalboni commented 7 years ago

It looks good, so far!

Regarding mixed-language projects, it would be cool! We should be able to easily detect both api.js and api.py, and parse/generate both on deploy.

lusentis commented 7 years ago

Okay, I've just discovered that docker-lambda uses different docker images for different runtimes, so we must pass a dockerImage option to dockerLambda (reference, our code)

alexcasalboni commented 7 years ago

Awesome!

But wait.. does it mean that we can't have both at the same time in dev?

lusentis commented 7 years ago

no :) currently docker-lambda is invoked once for each function execution and it does no container reuse (it's not supported). We just specify the docker-lambda image to use with each invocation. This is really easy to implement because we determine the image name from the "api.runtime" property!

Edit: implemented in ~10 lines of code 👍 (https://github.com/dawson-org/dawson-cli/pull/136/commits/ceafca03faa36828113f55c87a21e8ad86cd899a)

lusentis commented 7 years ago

I've cleaned up this branch:

🎉

alexcasalboni commented 7 years ago

Awesome. I've just opened a new PR to add the runtime config param.

lusentis commented 7 years ago

@alexcasalboni would you prefer to have write access to this branch?

alexcasalboni commented 7 years ago

Yeah, that would make things faster :)

lusentis commented 7 years ago

I've just pushed a basic language-python implementation and I'm able to correctly deploy python functions! There's still some work to do (clean up, mostly). Here's a py example I'm using: https://github.com/dawson-org/dawson-examples/tree/master/2-basic-python

lusentis commented 7 years ago

dawson dev now works too 🎉 (except for authorizers) @alexcasalboni feel free to update any code that needs to be improved etc (I'm not too familiar with python in particular)

alexcasalboni commented 7 years ago

Ok, great! I will have another look around and fix those paths asap.

alexcasalboni commented 7 years ago

@lusentis please have a look at the last few commits! I didn't have time to properly test the devInstrument for Python, but it should be a good starting point :)

lusentis commented 7 years ago

Thank you! I'll have a look this evening.

lusentis commented 7 years ago

(my Mac broke and I can't get this to work on Windows..., I'll need a few more days to review this)

stale[bot] commented 7 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

alexcasalboni commented 7 years ago

@lusentis do you have a new Mac now? ;)

lusentis commented 7 years ago

No, it's a PC 💻. I promise I'll work on this in the next days.

codecov[bot] commented 7 years ago

Codecov Report

Merging #136 into master will decrease coverage by 4.86%. The diff coverage is 26.31%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #136      +/-   ##
==========================================
- Coverage   92.94%   88.07%   -4.87%     
==========================================
  Files           9       10       +1     
  Lines         241      260      +19     
==========================================
+ Hits          224      229       +5     
- Misses         17       31      +14
Impacted Files Coverage Δ
src/factories/primaryTemplate.js 100% <ø> (ø) :arrow_up:
src/factories/cf_apig.js 100% <100%> (ø) :arrow_up:
src/libs/language-python/createIndex.js 6.66% <6.66%> (ø)

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 8702799...df2534b. Read the comment docs.

stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.