LGTM (40 Part Series)

1 LGTM Devlog 0: Teaching Git through playing a game
2 LGTM Devlog 1: New Logo, and jumping the gun by buying a domain
… 36 more parts…
3 LGTM Devlog 2: Design Ideas for the game I
4 LGTM Devlog 3: Design Ideas for the game 2
5 LGTM Devlog 4: Technical Decisions – GitHub API and Python
6 LGTM Devlog 5: Technical Decisions – Serverless Architecture
7 LGTM Devlog 6: Sprint 1 plans
8 LGTM Devlog 7: Creating a Firebase project
9 LGTM Devlog 8: Starting a git repo for an open source project from scratch
10 LGTM Devlog 9: Python Google Cloud Functions with Unit Tests and Linting
11 LGTM Devlog 10: Capturing the GitHub webhook for fork requests
12 LGTM Devlog 11: Writing the Serverless Function for receiving GitHub webhooks with Pydantic validation
13 LGTM Devlog 12: CI/CD with GitHub Actions to run Unit Tests and deploy Firebase Functions
14 LGTM Devlog 13: GitHub Branch Protection and Security
15 LGTM Devlog 14: Sprint 1 Retrospective
16 LGTM Devlog 15: Sprint 2 plans
17 LGTM Devlog 16: A serverless data base access rule conundrum
18 LGTM Devlog 17: Website and GitHub OAuth
19 LGTM Devlog 18: Python Serverless functions using GitHub API to validate users
20 LGTM Devlog 19: Game data/quest storage
21 LGTM Devlog 20: Python Abstract Base Class-based data/quest storage
22 LGTM Devlog 21: Deploying Pub/Sub-triggered Python Google Cloud Functions
23 LGTM Devlog 22: Modularization
24 LGTM Devlog 23: Sprint 2 Retrospective
25 LGTM Devlog 24: Sprint 3 plans
26 LGTM Devlog 25: Some cleanup
27 LGTM Devlog 26: Python Graphlib DAGs for Quest Stages
28 LGTM Devlog 27: Branching quests
29 LGTM Devlog 28: Game event loop using Google Cloud Scheduler and PubSub
30 LGTM Devlog 29: ORM for Firestore and __init__subclass__ dunders and metaclasses
31 LGTM Devlog 30: Sprint 3 Retrospective
32 LGTM Devlog 31: Sprint 4 Plan
33 LGTM Devlog 32: Secrets Management to avoid storing API keys in services
34 LGTM Devlog 33: Using PyGithub to post GitHub issues and comments
35 LGTM Devlog 34: Characters Posting on GitHub Issues
36 LGTM Devlog 35: Responding to the player’s answers on GitHub Issues Comments
37 LGTM Devlog 36: Character profiles! On GitHub
38 LGTM Devlog 37: Sprint 4 Retrospective
39 LGTM Devlog 38: Sprint 5 Plan
40 LGTM Devlog 39: Planning the story

I just realized my old branch wasn’t merged in yet, and as I do that, my CI flags up coverage failure, since I’d set the test coverage minimum to 95%, and was getting lower than that.

While you shouldn’t live life by these numbers, these are indicators and should be looked into. If I deem there’s a reason to allow it, I’d ignore it. The code for this post can be found at commit 48c6051

First, I’m going to add a quick script into my Pipfile called test_report to help look at coverage results. Since pycov generates HTML reports, this line of code makes my life easier by spitting out the HTML report and starting a temporary webserver with python to let me view it. If I weren’t using WSL, I could have this script pop open a web browser too


[scripts]
test = "pytest"
test_report = "bash -c 'pytest --cov-report=html && python -m http.server --directory htmlcov'"
deploy = "./deploy.sh"
[scripts]
test = "pytest"
test_report = "bash -c 'pytest --cov-report=html && python -m http.server --directory htmlcov'"
deploy = "./deploy.sh"
[scripts]
test = "pytest"
test_report = "bash -c 'pytest --cov-report=html && python -m http.server --directory htmlcov'"
deploy = "./deploy.sh"

Enter fullscreen mode Exit fullscreen mode

Now I can have a browsable file tree containing coverage results

I need to set up my VSCode extensions to display coverage results at some point. For now this is an easy way to view my report

firebase.py

Starting with Firebase.py, at 83% coverage, the reason is simple: I added in some conditional code to return the previously initialized firebase app/singleton, when in fact, putting this code in it’s own module achieved the same effect, so it never ends up going into the branch. And since it’s only 8 lines of code, drops the coverage by 12.5%.

I simply remove this whole if statement. Done

main.py

Over at the main function, the first lack of coverage is an error handling when the hook repo didn’t correctly match.

This indicates I’m missing a test where I present this function with a bad repo address. To adequately test this, I would need to generate some new hook payloads that are otherwise valid but have the wrong source repo. So I simply add an extra test case here.

In the next function, there’s a few things going on, firstly a missing CORS header check. I don’t have a test for CORS header handling. While I could add one, I’ve opted to instead eliminate this manual check and just use the flask-cors library that provides a decorator. Now instead of handling CORS manually, I let the decorator deal with it:


<span>from</span> <span>flask_cors</span> <span>import</span> <span>cross_origin</span>
<span>@</span><span>cross_origin</span><span>(</span>
    <span>origins</span><span>=</span><span>CORS_ORIGIN</span><span>,</span>
    <span>headers</span><span>=</span><span>[</span><span>"Authorization"</span><span>,</span> <span>"Content-Type"</span><span>],</span>
    <span>supports_credentials</span><span>=</span><span>True</span><span>,</span>
<span>)</span>
<span>def</span> <span>github_auth_flow</span><span>(</span><span>request</span><span>:</span> <span>Request</span><span>):</span>
    <span>""" Validates a user from github and creates user """</span>
    <span>...</span>
<span>from</span> <span>flask_cors</span> <span>import</span> <span>cross_origin</span>

<span>@</span><span>cross_origin</span><span>(</span>
    <span>origins</span><span>=</span><span>CORS_ORIGIN</span><span>,</span>
    <span>headers</span><span>=</span><span>[</span><span>"Authorization"</span><span>,</span> <span>"Content-Type"</span><span>],</span>
    <span>supports_credentials</span><span>=</span><span>True</span><span>,</span>
<span>)</span>
<span>def</span> <span>github_auth_flow</span><span>(</span><span>request</span><span>:</span> <span>Request</span><span>):</span>
    <span>""" Validates a user from github and creates user """</span>
    <span>...</span>
from flask_cors import cross_origin

@cross_origin(
    origins=CORS_ORIGIN,
    headers=["Authorization", "Content-Type"],
    supports_credentials=True,
)
def github_auth_flow(request: Request):
    """ Validates a user from github and creates user """
    ...

Enter fullscreen mode Exit fullscreen mode

The next item relates to handling when the provided user ID does not match the user ID that github returns as a token, this may happen in a spoofing attempt for example. For this I add another test that manipulates the user Id data provided to the function so that it is incorrect.


<span>def</span> <span>test_id_mismatch</span><span>(</span><span>auth_flow_client</span><span>,</span> <span>test_user_token</span><span>,</span> <span>user_data</span><span>):</span>
    <span>""" Test a successful flow """</span>
    <span>user_data</span><span>[</span><span>"id"</span><span>]</span> <span>=</span> <span>"foobar"</span>  <span># make id bad </span>    <span>res</span> <span>=</span> <span>auth_flow_client</span><span>.</span><span>post</span><span>(</span>
        <span>"/"</span><span>,</span> <span>headers</span><span>=</span><span>{</span><span>"Authorization"</span><span>:</span> <span>"Bearer "</span> <span>+</span> <span>test_user_token</span><span>},</span> <span>json</span><span>=</span><span>user_data</span>
    <span>)</span>
    <span>assert</span> <span>res</span><span>.</span><span>status_code</span> <span>==</span> <span>403</span>
<span>def</span> <span>test_id_mismatch</span><span>(</span><span>auth_flow_client</span><span>,</span> <span>test_user_token</span><span>,</span> <span>user_data</span><span>):</span>
    <span>""" Test a successful flow """</span>

    <span>user_data</span><span>[</span><span>"id"</span><span>]</span> <span>=</span> <span>"foobar"</span>  <span># make id bad </span>    <span>res</span> <span>=</span> <span>auth_flow_client</span><span>.</span><span>post</span><span>(</span>
        <span>"/"</span><span>,</span> <span>headers</span><span>=</span><span>{</span><span>"Authorization"</span><span>:</span> <span>"Bearer "</span> <span>+</span> <span>test_user_token</span><span>},</span> <span>json</span><span>=</span><span>user_data</span>
    <span>)</span>
    <span>assert</span> <span>res</span><span>.</span><span>status_code</span> <span>==</span> <span>403</span>
def test_id_mismatch(auth_flow_client, test_user_token, user_data):
    """ Test a successful flow """

    user_data["id"] = "foobar"  # make id bad     res = auth_flow_client.post(
        "/", headers={"Authorization": "Bearer " + test_user_token}, json=user_data
    )
    assert res.status_code == 403

Enter fullscreen mode Exit fullscreen mode

Finally, the last remaining line not covered by testing is the event where a game already exists so that a user can be assigned. To achieve this, I need to go and create a new game for the user before anything else, so the test fixture now looks like this:


<span>def</span> <span>test_good_flow</span><span>(</span><span>auth_flow_client</span><span>,</span> <span>test_user_token</span><span>,</span> <span>user_data</span><span>,</span> <span>test_auth_user</span><span>):</span>
    <span>""" Test a successful flow """</span>
    <span># make sure a game exists for user </span>    <span>user</span> <span>=</span> <span>User</span><span>.</span><span>reference</span><span>(</span><span>Source</span><span>.</span><span>GITHUB</span><span>,</span> <span>user_data</span><span>[</span><span>"id"</span><span>])</span>
    <span>game</span> <span>=</span> <span>Game</span><span>.</span><span>new</span><span>(</span><span>user</span><span>,</span> <span>"fork_url"</span><span>)</span>
    <span>res</span> <span>=</span> <span>auth_flow_client</span><span>.</span><span>post</span><span>(</span>
        <span>"/"</span><span>,</span> <span>headers</span><span>=</span><span>{</span><span>"Authorization"</span><span>:</span> <span>"Bearer "</span> <span>+</span> <span>test_user_token</span><span>},</span> <span>json</span><span>=</span><span>user_data</span>
    <span>)</span>
    <span>assert</span> <span>res</span><span>.</span><span>status_code</span> <span>==</span> <span>200</span>
    <span># check firestore </span>    <span>doc</span> <span>=</span> <span>db</span><span>.</span><span>collection</span><span>(</span><span>"users"</span><span>).</span><span>document</span><span>(</span><span>test_auth_user</span><span>.</span><span>uid</span><span>).</span><span>get</span><span>()</span>
    <span>assert</span> <span>doc</span><span>.</span><span>exists</span>
    <span>assert</span> <span>doc</span><span>.</span><span>get</span><span>(</span><span>"id"</span><span>)</span> <span>==</span> <span>user_data</span><span>[</span><span>"id"</span><span>]</span>
    <span># cleanup </span>    <span>db</span><span>.</span><span>collection</span><span>(</span><span>"game"</span><span>).</span><span>document</span><span>(</span><span>game</span><span>.</span><span>key</span><span>).</span><span>delete</span><span>()</span>
    <span>db</span><span>.</span><span>collection</span><span>(</span><span>"system"</span><span>).</span><span>document</span><span>(</span><span>"stats"</span><span>).</span><span>update</span><span>({</span><span>"games"</span><span>:</span> <span>firestore</span><span>.</span><span>Increment</span><span>(</span><span>-</span><span>1</span><span>)})</span>
    <span># cleanup auto-created quest too </span>    <span>QuestClass</span> <span>=</span> <span>Quest</span><span>.</span><span>get_first</span><span>()</span>
    <span>quest</span> <span>=</span> <span>QuestClass</span><span>()</span>
    <span>quest</span><span>.</span><span>game</span> <span>=</span> <span>game</span>
    <span>db</span><span>.</span><span>collection</span><span>(</span><span>"quest"</span><span>).</span><span>document</span><span>(</span><span>quest</span><span>.</span><span>key</span><span>).</span><span>delete</span><span>()</span>
<span>def</span> <span>test_good_flow</span><span>(</span><span>auth_flow_client</span><span>,</span> <span>test_user_token</span><span>,</span> <span>user_data</span><span>,</span> <span>test_auth_user</span><span>):</span>
    <span>""" Test a successful flow """</span>

    <span># make sure a game exists for user </span>    <span>user</span> <span>=</span> <span>User</span><span>.</span><span>reference</span><span>(</span><span>Source</span><span>.</span><span>GITHUB</span><span>,</span> <span>user_data</span><span>[</span><span>"id"</span><span>])</span>
    <span>game</span> <span>=</span> <span>Game</span><span>.</span><span>new</span><span>(</span><span>user</span><span>,</span> <span>"fork_url"</span><span>)</span>

    <span>res</span> <span>=</span> <span>auth_flow_client</span><span>.</span><span>post</span><span>(</span>
        <span>"/"</span><span>,</span> <span>headers</span><span>=</span><span>{</span><span>"Authorization"</span><span>:</span> <span>"Bearer "</span> <span>+</span> <span>test_user_token</span><span>},</span> <span>json</span><span>=</span><span>user_data</span>
    <span>)</span>
    <span>assert</span> <span>res</span><span>.</span><span>status_code</span> <span>==</span> <span>200</span>

    <span># check firestore </span>    <span>doc</span> <span>=</span> <span>db</span><span>.</span><span>collection</span><span>(</span><span>"users"</span><span>).</span><span>document</span><span>(</span><span>test_auth_user</span><span>.</span><span>uid</span><span>).</span><span>get</span><span>()</span>
    <span>assert</span> <span>doc</span><span>.</span><span>exists</span>
    <span>assert</span> <span>doc</span><span>.</span><span>get</span><span>(</span><span>"id"</span><span>)</span> <span>==</span> <span>user_data</span><span>[</span><span>"id"</span><span>]</span>

    <span># cleanup </span>    <span>db</span><span>.</span><span>collection</span><span>(</span><span>"game"</span><span>).</span><span>document</span><span>(</span><span>game</span><span>.</span><span>key</span><span>).</span><span>delete</span><span>()</span>
    <span>db</span><span>.</span><span>collection</span><span>(</span><span>"system"</span><span>).</span><span>document</span><span>(</span><span>"stats"</span><span>).</span><span>update</span><span>({</span><span>"games"</span><span>:</span> <span>firestore</span><span>.</span><span>Increment</span><span>(</span><span>-</span><span>1</span><span>)})</span>

    <span># cleanup auto-created quest too </span>    <span>QuestClass</span> <span>=</span> <span>Quest</span><span>.</span><span>get_first</span><span>()</span>
    <span>quest</span> <span>=</span> <span>QuestClass</span><span>()</span>
    <span>quest</span><span>.</span><span>game</span> <span>=</span> <span>game</span>
    <span>db</span><span>.</span><span>collection</span><span>(</span><span>"quest"</span><span>).</span><span>document</span><span>(</span><span>quest</span><span>.</span><span>key</span><span>).</span><span>delete</span><span>()</span>
def test_good_flow(auth_flow_client, test_user_token, user_data, test_auth_user):
    """ Test a successful flow """

    # make sure a game exists for user     user = User.reference(Source.GITHUB, user_data["id"])
    game = Game.new(user, "fork_url")

    res = auth_flow_client.post(
        "/", headers={"Authorization": "Bearer " + test_user_token}, json=user_data
    )
    assert res.status_code == 200

    # check firestore     doc = db.collection("users").document(test_auth_user.uid).get()
    assert doc.exists
    assert doc.get("id") == user_data["id"]

    # cleanup     db.collection("game").document(game.key).delete()
    db.collection("system").document("stats").update({"games": firestore.Increment(-1)})

    # cleanup auto-created quest too     QuestClass = Quest.get_first()
    quest = QuestClass()
    quest.game = game
    db.collection("quest").document(quest.key).delete()

Enter fullscreen mode Exit fullscreen mode

There’s a lot of cleanup in here. I can reduce a lot of this by re-using the fixtures already defined for creating users. I will do so in a future tidy-up, particularly if I can get the local Firestore emulator working so that I no longer have to test with the actual database (and therefore don’t have to worry about cleaning up after test data).

Automatically handle exceptions

Firebase’s python functions framework has a way of registering exception handlers to automatically run to return values on exception, rather than having to explicitly return a json object. We can use this to keep the code tidy. For now, I will add a single exception handler for dealing with pydantic validation failures:


<span>@</span><span>errorhandler</span><span>(</span><span>ValidationError</span><span>)</span>
<span>def</span> <span>validation_error</span><span>(</span><span>err</span><span>:</span> <span>ValidationError</span><span>):</span>
    <span>""" Handler for pydantic validation errors (usuall http payload) """</span>
    <span>logger</span><span>.</span><span>error</span><span>(</span><span>"Validation error"</span><span>,</span> <span>err</span><span>=</span><span>err</span><span>)</span>
    <span>return</span> <span>jsonify</span><span>(</span><span>error</span><span>=</span><span>"Validation error"</span><span>),</span> <span>400</span>
<span>@</span><span>errorhandler</span><span>(</span><span>ValidationError</span><span>)</span>
<span>def</span> <span>validation_error</span><span>(</span><span>err</span><span>:</span> <span>ValidationError</span><span>):</span>
    <span>""" Handler for pydantic validation errors (usuall http payload) """</span>
    <span>logger</span><span>.</span><span>error</span><span>(</span><span>"Validation error"</span><span>,</span> <span>err</span><span>=</span><span>err</span><span>)</span>
    <span>return</span> <span>jsonify</span><span>(</span><span>error</span><span>=</span><span>"Validation error"</span><span>),</span> <span>400</span>
@errorhandler(ValidationError)
def validation_error(err: ValidationError):
    """ Handler for pydantic validation errors (usuall http payload) """
    logger.error("Validation error", err=err)
    return jsonify(error="Validation error"), 400

Enter fullscreen mode Exit fullscreen mode

Now, within each of the functions, I no longer have to explicitly handle the pydantic validationError, as the framework will run this function for me when that exception occurs.

Dependency Injection

One thing I really miss about using FastAPI, and having to work with this kind of weird Flask-like implementation, is how FastAPI handle’s dependency injection. So… let’s roll our own. I use some of Python’s introspection methods to detect whether there’s a Pydantic model defined as any of the function arguments’ type hints, and do a decode of the payload and inject it into the function, using a decorator:


<span>def</span> <span>inject_pydantic_parse</span><span>(</span><span>func</span><span>):</span>
    <span>""" Wrap method with pydantic dependency injection """</span>
    <span>def</span> <span>wrapper</span><span>(</span><span>request</span><span>:</span> <span>Request</span><span>):</span>
        <span>kwargs</span> <span>=</span> <span>{}</span>
        <span>for</span> <span>arg_name</span><span>,</span> <span>arg_type</span> <span>in</span> <span>get_type_hints</span><span>(</span><span>func</span><span>).</span><span>items</span><span>():</span>
            <span>parse_raw</span> <span>=</span> <span>getattr</span><span>(</span><span>arg_type</span><span>,</span> <span>"parse_raw"</span><span>,</span> <span>None</span><span>)</span>
            <span>if</span> <span>callable</span><span>(</span><span>parse_raw</span><span>):</span>
                <span>kwargs</span><span>[</span><span>arg_name</span><span>]</span> <span>=</span> <span>parse_raw</span><span>(</span><span>request</span><span>.</span><span>data</span><span>)</span>
                <span>logger</span><span>.</span><span>info</span><span>(</span>
                    <span>"Decoded model and injected"</span><span>,</span>
                    <span>model</span><span>=</span><span>arg_type</span><span>.</span><span>__name__</span><span>,</span>
                    <span>func</span><span>=</span><span>func</span><span>.</span><span>__name__</span><span>,</span>
                <span>)</span>
        <span>return</span> <span>func</span><span>(</span><span>request</span><span>,</span> <span>**</span><span>kwargs</span><span>)</span>
    <span>return</span> <span>wrapper</span>
<span>@</span><span>inject_pydantic_parse</span>
<span>def</span> <span>github_webhook_listener</span><span>(</span><span>request</span><span>:</span> <span>Request</span><span>,</span> <span>hook_fork</span><span>:</span> <span>GitHubHookFork</span><span>):</span>
    <span>""" A listener for github webhooks """</span>
    <span>...</span>
<span>def</span> <span>inject_pydantic_parse</span><span>(</span><span>func</span><span>):</span>
    <span>""" Wrap method with pydantic dependency injection """</span>

    <span>def</span> <span>wrapper</span><span>(</span><span>request</span><span>:</span> <span>Request</span><span>):</span>
        <span>kwargs</span> <span>=</span> <span>{}</span>
        <span>for</span> <span>arg_name</span><span>,</span> <span>arg_type</span> <span>in</span> <span>get_type_hints</span><span>(</span><span>func</span><span>).</span><span>items</span><span>():</span>
            <span>parse_raw</span> <span>=</span> <span>getattr</span><span>(</span><span>arg_type</span><span>,</span> <span>"parse_raw"</span><span>,</span> <span>None</span><span>)</span>
            <span>if</span> <span>callable</span><span>(</span><span>parse_raw</span><span>):</span>
                <span>kwargs</span><span>[</span><span>arg_name</span><span>]</span> <span>=</span> <span>parse_raw</span><span>(</span><span>request</span><span>.</span><span>data</span><span>)</span>
                <span>logger</span><span>.</span><span>info</span><span>(</span>
                    <span>"Decoded model and injected"</span><span>,</span>
                    <span>model</span><span>=</span><span>arg_type</span><span>.</span><span>__name__</span><span>,</span>
                    <span>func</span><span>=</span><span>func</span><span>.</span><span>__name__</span><span>,</span>
                <span>)</span>

        <span>return</span> <span>func</span><span>(</span><span>request</span><span>,</span> <span>**</span><span>kwargs</span><span>)</span>

    <span>return</span> <span>wrapper</span>


<span>@</span><span>inject_pydantic_parse</span>
<span>def</span> <span>github_webhook_listener</span><span>(</span><span>request</span><span>:</span> <span>Request</span><span>,</span> <span>hook_fork</span><span>:</span> <span>GitHubHookFork</span><span>):</span>
    <span>""" A listener for github webhooks """</span>
    <span>...</span>
def inject_pydantic_parse(func):
    """ Wrap method with pydantic dependency injection """

    def wrapper(request: Request):
        kwargs = {}
        for arg_name, arg_type in get_type_hints(func).items():
            parse_raw = getattr(arg_type, "parse_raw", None)
            if callable(parse_raw):
                kwargs[arg_name] = parse_raw(request.data)
                logger.info(
                    "Decoded model and injected",
                    model=arg_type.__name__,
                    func=func.__name__,
                )

        return func(request, **kwargs)

    return wrapper


@inject_pydantic_parse
def github_webhook_listener(request: Request, hook_fork: GitHubHookFork):
    """ A listener for github webhooks """
    ...

Enter fullscreen mode Exit fullscreen mode

In the above example, github_webhook_listener() has been wrapped by my new inject_pydantic_parse() function, which will detect that there is an extra hook_fork: GitHubHookFork argument, and since GitHubHookFork has a parse_raw callable, it will run request.data through it, and inject it as that argument.

Now, whenever one of my functions expects a json payload that I have a pydantic model for, I just specify it as an argument whose type hint is a pydantic object (or any class with a `parse_raw()’ method, thanks to duck-typing). This keeps the code clean of duplicate pydantic model decoding and error handling.

And now, the CI reports 100% test coverage!

LGTM (40 Part Series)

原文链接：LGTM Devlog 25: Some cleanup

展开阅读全文

文章版权声明 1、本网站名称：拾光赋
2、本站永久网址：https://www.blogs.ink
3、本网站的文章部分内容可能来源于网络，仅供大家学习与参考，如有侵权，请联系站长QQ：805375623进行删除处理。
4、本站一切资源不代表本站立场，并不代表本站赞同其观点和对其真实性负责。
5、本站一律禁止以任何方式发布或转载任何违法的相关信息，访客发现请向站长举报
6、本站资源大多存储在云盘，如发现链接失效，请联系我们我们会第一时间更新。

THE END