Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pimp my ride #2

Open
wants to merge 16 commits into
base: master
from
Open

Pimp my ride #2

wants to merge 16 commits into from

Conversation

@revmischa
Copy link

@revmischa revmischa commented Nov 9, 2018

Yo dawg we got API endpoints that document themselves and a CORS plugin in the trunk you gonna love
We're using yarn cause npm is played out
Kicking that old python 2 nonsense to the curb
And it comes pre-loaded with a swagger GUI thats fresher than a lemonade stand

Also: most people don't need to dockerize pip and if they do they know they do and can edit the config in serverless.yml.

pimpmyride

@revmischa
Copy link
Author

@revmischa revmischa commented Nov 10, 2018

I opened a PR to fix the swagger paths when it's running under /dev, /prod, etc
https://github.com/jmcarp/flask-apispec/pull/125/files

@alexdebrie
Copy link
Owner

@alexdebrie alexdebrie commented Nov 18, 2018

Hey @revmischa, thanks for the PR. Lots of good stuff here.

I do think it's a little more opinionated than I want for this template. Not everyone will want an OpenAPI. I also think that some people will want a choice between Python2 and Python3. As for dockerizing pip, the reason is if you use packages that need to compile C extensions, like SQLAlchemy or some of the data science stack. I'd rather that be baked in pretty easily rather than have people search for workarounds. As for yarn vs npm, I'm pretty indifferent given that it's a pretty small part of the project.

Given that, I think I'd prefer to keep this template more generic. If there are certain changes that are more generic, I'm happy to take those. For a more opinionated template around OpenAPI, it'd probably be best to create your own fork or start from scratch.

Let me know if you have any questions or would like to discuss!

@revmischa
Copy link
Author

@revmischa revmischa commented Nov 19, 2018

That's all fair.

I'm looking into options now for GraphQL which would obviate the need for OpenAPI.

People should be discouraged from using python 2 IMO

@revmischa
Copy link
Author

@revmischa revmischa commented Nov 21, 2018

I restored the docker question.
Using OpenAPI isn't very opinionated because you don't actually have to do anything. It just comes free as a bonus if you're already using marshmallow. Marshmallow is pretty essential to most kinds of APIs, and I figure you aren't making a server-side template driven web application if you're making a serverless app. That was just my thinking anyway. OpenAPI comes for free so why not add it, and codegen comes free with OpenAPI so why not add that option too.

@seanpue
Copy link

@seanpue seanpue commented Aug 5, 2019

Thanks @revmischa (and @alexdebrie!) this is super helpful and really exciting!

@revmischa, this branch is currently not working({"message": "Internal server error"}). There seems to be some sort of problem with the fixed flask_apispec missing the 'openapi_version' for the MarshmallowPlugin (when I used venv). When I changed requirements.txt to use the latest version of flask_apispec, then it worked locally and loaded on AWS but OpenAPI was not curling the AWS /dev/ path but instead /api/ (without the /dev/ prefix).

Also, the services are all showing up as '/undefined' in the Serverless dashboard. I am not sure how to fix that.

If you have any suggestions on how to fix those issues I would be very grateful for a response. Cheers!

@revmischa
Copy link
Author

@revmischa revmischa commented Aug 5, 2019

Oh this is super old. I've been developing this instead, based off the idea of this repo: https://github.com/jetbridge/sls-flask/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants