feat: Add mypy type checking

[zachaysan]

Changes

This adds mypy type checking. Existing failures have been grandfathered in and a helper script is run to compare failures against the predefined list. Mypy was also added into the pre-commit config and will run on any commits to python code.

How did you test this code?

I altered the predefined mypy list and saw a failure crop up when running pre-commit.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

3 Skipped Deployments

Name	Status	Preview	Comments	Updated (UTC)
docs	⬜️ Ignored (Inspect)	Visit Preview		Dec 17, 2024 4:47pm
flagsmith-frontend-preview	⬜️ Ignored (Inspect)	Visit Preview		Dec 17, 2024 4:47pm
flagsmith-frontend-staging	⬜️ Ignored (Inspect)	Visit Preview		Dec 17, 2024 4:47pm

[github-actions]

Docker builds report

Image	Build Status	Security report
ghcr.io/flagsmith/flagsmith-api-test:pr-4863	Finished ✅	Skipped
ghcr.io/flagsmith/flagsmith-e2e:pr-4863	Finished ✅	Skipped
ghcr.io/flagsmith/flagsmith-frontend:pr-4863	Finished ✅	Results ✅
ghcr.io/flagsmith/flagsmith-api:pr-4863	Finished ✅	Results ✅
ghcr.io/flagsmith/flagsmith:pr-4863	Finished ✅	Results ✅
ghcr.io/flagsmith/flagsmith-private-cloud:pr-4863	Finished ✅	Results ✅

[github-actions]

Uffizzi Ephemeral Environment deployment-58551

☁️ https://app.uffizzi.com/github.com/Flagsmith/flagsmith/pull/4863

📄 View Application Logs etc.

What is Uffizzi? Learn more!

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.39%. Comparing base (05ab7cf) to head (6dcff38).
Report is 29 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #4863    +/-   ##
========================================
  Coverage   97.38%   97.39%            
========================================
  Files        1189     1191     +2     
  Lines       41420    41526   +106     
========================================
+ Hits        40338    40444   +106     
  Misses       1082     1082            

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[matthewelwell]

This seems like a decent enough solution to me but I have some general questions:

1. What happens if we (perhaps inadvertently) resolve an existing issue. Based on the logic in your script, I guess we wouldn't know about it, right? Because we're just seeing what's left after we subtract the original errors from the new ones which wouldn't tell us anything about any that we've solved. It would obviously be great to incentivise resolving existing issues where we can.

2. This approach works fine I guess, but another solution would have been to just add all of the distinct files into the exclude. Then every new file would need to conform to mypy, but changes to existing files wouldn't. I'm not sure how we could incentivise fixing the existing errors. (Note: I'm certainly not suggesting this is a better option, I'm just playing devil's advocate).

[matthewelwell]

Since I was looking at similar functionality yesterday (see here) , I am somewhat intrigued by this.

My understanding of pre-commit is that it passes the modified files into the command specified in entry, so the resulting command would look something like:

bash -c "cd api && poetry run python scripts/mypy_check.py" file1.py file2.py 

... which I would expect to just not work at all, or at least throw some sort of error?

[matthewelwell]

@zachaysan I'm not sure the latest changes make any reference to this?

[zachaysan]

I've altered the script to run the files one at a time. I had to use git diff to list the target files of the commit because no matter what I tried to pass into the bash script it just wouldn't run with the automatic listed files. The good news is that we are now doing a mypy check on only the committed files.