Make MyPy happy

requested review from @moser

assigned to @karp

Looks great, thank you very much for your attention to detail!

I am not particularly happy about the alleged LSP violation. Why I understand why it is happening from a typing point of view, the fix doesn't feel ideal to me: You cannot set anything else than a datetime (or None) to a UtcDateTime field, so I don't think we should silently fail in this case.

Maybe SQLAlchemy can already handle this on its own? I'll have to think about this.

If you want to, you can open an MR containing everything but the two changes you commented on, which I will quickly approve and merge :)

mentioned in merge request !23

added 1 commit

de55edc5 - Fix typing in UtcDateTime and Uuid

Compare with previous version

resolved all threads

added 1 commit

3098ca66 - Add some more descriptive type aliases

Compare with previous version

added 1 commit

7fa961af - Use Optional instead of | None for consistency

Compare with previous version

added 1 commit

e2e69b26 - Make annotation for parameter more specific

Compare with previous version

added 1 commit

44d9fde1 - Remove likely unneeded annotation

Compare with previous version

added 1 commit

59bbca69 - Make annotation more specific

Compare with previous version

@karp I am currently reviewing the code, looks great! I made a few additions to the typing.

Now, I will still need to review auth.py and UtcDateTime.py for semantic reasons (I am always a bit overcareful when it comes to auth, and I don't quite understand what I did with UtcDateTime anymore), but I will merge your suggestions later this day.

added 1 commit

4dcb6d31 - Use Optional, remove incorrect type

Compare with previous version

added 2 commits

c40f2ff0 - Rearrange lines (first "assert" authorization type, then check payload)
e1c153f0 - Route may not necessarily return a ProblemResponse

Compare with previous version

approved this merge request

mentioned in commit 533884ec

merged

Make MyPy happy

Merge request reports

Activity