Added an adapter for FastAPI

[LeOndaz]

This PR introduces an adapter for FastAPI. I'll create an example and PR it later.

[jensens]

Thanks, this is great! Could you provide an example usage with test? like here: https://github.com/authomatic/authomatic/tree/master/examples

[LeOndaz]

Thanks, this is great! Could you provide an example usage with test? like here: https://github.com/authomatic/authomatic/tree/master/examples

Thank you. I'll soon enough

[LeOndaz]

@jensens I think I'm having some problems running the tests, after I installed the deps, I run tox in the project directory and

py3 run-test: commands[0] | pylint --errors-only --ignore=six.py authomatic
************* Module authomatic.extras.flask
authomatic/extras/flask.py:47:8: E0237: Assigning to attribute 'modified' not defined in class slots (assigning-non-slot)
ERROR: InvocationError for command /home/leondaz/contrib/authomatic/.tox/py3/bin/pylint --errors-only --ignore=six.py authomatic (exited with code 2)
____________________________________________________________________________________________________________ summary ____________________________________________________________________________________________________________
ERROR:   py3: commands failed

I'm running Python 3.8

[LeOndaz]

@jensens I understand the error, but I've assumed that this error happened only to me because if authomatic is on GitHub with TravisCI then the tests should be passing right?

[mrichar1]

I haven't had a deep dig, but it looks like the authomatic code calls session.modified = True (wheresession is imported from flask. My guess is that the flask api has changed in some way. I'll have a read up on the flask docs and see if this line needs modified or removed.

[mrichar1]

OK so it looks like this is still the 'right thing to do' according to the Flask docs: https://flask.palletsprojects.com/en/2.0.x/api/#sessions

It looks like this is probably a bug in flask (something like they define __slots__ but don't have modified in it). It's probably safe to have a pylint 'ignore' for this line, but we should open a bug with Flask and get them to confirm this.

@LeOndaz if you want to work round this for now, just chang the last line in authomatic/extras/flask.py to be:

        session.modified = True  # pylint: disable=assigning-non-slot

(but don't commit this in any PRs you make, and we'll hopefully resolve it separately).

[mrichar1]

( @jensens Raised with Flask in pallets/flask#4219 - will see what they say)

[LeOndaz]

@mrichar1 Thank you for your efforts, not gonna lie, I removed flask support when I got the error 😂

[LeOndaz]

@jensens @mrichar1 Kindly, review when you have time. And thanks again for your efforts, I appreciate.

[LeOndaz]

That's it for today. Sorry for missing the formatting thing, I didn't notice that there's an unformatted file.

[mrichar1]

Flask have confirmed the pylint issue is an upstream pylint false-positive, so have pushed the pylint disable comment to master to hide this error.

@LeOndaz It looks liek this has raised a conflict, so you'll need to fix/rebase your PR against master.

[LeOndaz]

@mrichar1 I've fixed the conflict, you've added the comment and I've disabled pylint for the line

[LeOndaz]

@mrichar1 Hello, can you review and merge if it's okay with you?

[mrichar1]

We've just been making changes to re-enable automatic testing on PRs and commits - I've just merged these chanegs in master onto this PR to get these tests running. Will then start a code review when they complete.

[LeOndaz]

@mrichar1 Perfect, Thanks for your efforts. we've now 4 checks that passed.

[mrichar1]

LGTM 👍

[mrichar1]

I had missed that while fastAPI was added to the test stack, it wasn't actually being selected in the test config. I've updated this setting in your branch, which should cause it to run - though the tests that require auth secrets will be skipped. I'll run these offline and feed back if there are any issues.

[mrichar1]

So the test with secrets failed as liveandletdie is failing to start FastAPIServer. It looks like this is down to liveandletdie not starting uvicorn/the example app properly. This is down to a few things:

• uvicorn needs a app-dir parameter to define the path that the module is loaded from - so the long path needs splitting out into pth and module.
• There's no app or equivalent 'start-point' in the example app for uvicorn to load (i.e. main:main` or equivalent).
• There might be issues if these methods aren't defined as asynchronous.

Can you look at getting the test suite updated in liveandletdie to start uvicorn and run the test app correctly? This will then let us fully test this PR.

[LeOndaz]

@mrichar1
I'll be investigating and get back to you

[LeOndaz]

@mrichar1
I think all of the points hold in the PR, except the fact that I forgot to remove the .py from "app.py:app" and I think this is why it didn't work. Can you have a look at the other repo?

[LeOndaz]

@mrichar1 Kindly review the other repo PRs to solve and merge this one

[LeOndaz]

@mrichar1 I'll appreciate if you reply

[LeOndaz]

@mrichar1 hopefully you're still alive 😅

[jensens]

@LeOndaz I tried your branch as we need FastAPI support in our project. The example does not work here. I made my own enhanced example for the project here https://github.com/ECC-Pilot/example-fastapi-github-oauth2

I'll prepare a review with fixes.

[jensens]

findings

[LeOndaz]

Feel free to commit in this PR and merge after you are done, I'm no longer interested in authomatic as it's unmaintained so I won't be exerting effort for it

[jensens]

@mrichar1 this is green now. So, nothing stops us from merging, right?

[mrichar1]

Yes - I finally managed to get tests and releases working again (!), so if it's passing then it should be fine to merge.

To make a new release, update the version in setup.cfg, and push to master tagged with the version number, and a new release should appear on PyPi. 🤞

(A push to master will run the full set of tests, including the functional oauth2 tests against Facebook, so might be best to push, then if that passes, push the tag after).

[jensens]

To make a new release, update the version in setup.cfg, and push to master tagged with the version number, and a new release should appear on PyPi. crossed_fingers

(A push to master will run the full set of tests, including the functional oauth2 tests against Facebook, so might be best to push, then if that passes, push the tag after).

The new release process has a glitch, see #225

[mrichar1]

It looks like the 'full' run of this the test suite for this PR is failing, with none of the FastAPI tests passing. The issue seems to resolve around liveandletdie not starting the FastAPI server correctly:

liveandletdie.LiveAndLetDieError: FastAPIServer server http://authomatic.org:443/ didn't start in specified timeout 10.0 seconds!

I can't remember what state FastAPI support in liveandletdie got to, if at all?

If we think that this code is valid, but the test is failing due to a lack of support in liveandletdie then we can leave this commit in, but remove fastapi from test_providers.py to prevent it trying and failing to test it - at least until we can patch liveandletdie appropriately...

[jensens]

I never dig into liveandletdie much. But that's probably the issue.