Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix mypy failures in main caused by updated click library #3745

Closed
adamsachs opened this issue Jul 7, 2023 · 2 comments · Fixed by #3746
Closed

Fix mypy failures in main caused by updated click library #3745

adamsachs opened this issue Jul 7, 2023 · 2 comments · Fixed by #3746
Assignees
Labels
bug Something isn't working

Comments

@adamsachs
Copy link
Contributor

adamsachs commented Jul 7, 2023

Bug Description

Changes introduced in the newly released click version 8.1.4, which now gets pulled into freshly baked images as a dependency, are seemingly causing mypy failures (e.g. https://github.com/ethyca/fides/actions/runs/5487381560/jobs/9998767180) in our CI runs. From an initial analysis, it seems like these changes specifically are what's responsible for the new mypy failures.

UPDATE: found pallets/click#2558 which confirms the above, and also that many others are seeing this same problem.

#3749 is a follow-up tech debt ticket that can be worked on if/when the underlying click issue above is fixed and released.

Steps to Reproduce

  1. nox -s clean
  2. nox -s dev -- shell
  3. in the container shell, run mypy and notice failures

Expected behavior

mypy should run successfully on our codebase! :)

Environment

The only thing to note here is that local docker cache can seemingly prevent this from appearing by not getting the latest click version and relying on a cached, older version instead. Running nox -s clean clears out this cache, and allows the error to be reproducible.

Most importantly, the error is consistently reproducible on CI runs.

Additional context

This is purely a CI/static check bug, so although it's annoying, it doesn't impact application runtime (as far as i can tell). Still, we should get it fixed.

@adamsachs adamsachs added the bug Something isn't working label Jul 7, 2023
@adamsachs adamsachs self-assigned this Jul 7, 2023
@adamsachs
Copy link
Contributor Author

adamsachs commented Jul 7, 2023

OK, now seeing that this looks to be heavily reported here: pallets/click#2558

looks like a fix is in progress. in the meantime, i propose we ignore the type checking at the relevant spots in our codebase (as is suggested by a click library maintainer ) and #3749 is in place as a tech debt ticket to revert if/when the typing issue is fixed in click

@Roger-Ethyca
Copy link
Contributor

moving to done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants