Skip to content

fix #486, add Starlette support#497

Merged
wwwjfy merged 3 commits intomasterfrom
starlette
Jun 6, 2019
Merged

fix #486, add Starlette support#497
wwwjfy merged 3 commits intomasterfrom
starlette

Conversation

@wwwjfy
Copy link
Copy Markdown
Member

@wwwjfy wwwjfy commented Jun 3, 2019

No description provided.

@coveralls
Copy link
Copy Markdown

coveralls commented Jun 3, 2019

Pull Request Test Coverage Report for Build 1751

  • 178 of 180 (98.89%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 98.573%

Changes Missing Coverage Covered Lines Changed/Added Lines %
gino/ext/starlette.py 83 84 98.81%
tests/test_starlette.py 89 90 98.89%
Totals Coverage Status
Change from base Build 1732: 0.01%
Covered Lines: 4215
Relevant Lines: 4276

💛 - Coveralls

@leosussan
Copy link
Copy Markdown

leosussan commented Jun 6, 2019

Thank you for taking this on Tony - this is awesome.

Testing this out with FastAPI (object inherits the Starlette model, so this should work for it). Runs very well locally - will troubleshoot a bit (doesn't yet fix my issue with Cloud Run, but that's an issue for another day & I'm pretty sure that has more to do with asyncpg than Gino).

It does require a bit of reorganization of my code (I define db in my models & import to main.py, which this isn't happy with unless route imports come below db definition) but this is not the end of the world.
Works really well after you do this!

@wwwjfy
Copy link
Copy Markdown
Member Author

wwwjfy commented Jun 6, 2019

Glad to hear that.
I tried to implement a "pure" middleware for that, but there are a few preconditions making that a bit more complex, so I end up with the current one.

Let us know if you have other ideas how to organise it and make it more user friendly. :)

@fantix
Copy link
Copy Markdown
Member

fantix commented Jun 6, 2019

Please feel free to merge - I'm sorry I don't have time to review recently. Thanks a lot for the good work!

@wwwjfy wwwjfy merged commit ab5a68a into master Jun 6, 2019
@wwwjfy wwwjfy deleted the starlette branch June 6, 2019 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants