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

ngclient: type annotations review and mypy integration #1409

Closed
jku opened this issue May 21, 2021 · 3 comments · Fixed by #1489
Closed

ngclient: type annotations review and mypy integration #1409

jku opened this issue May 21, 2021 · 3 comments · Fixed by #1489
Assignees
Labels
Milestone

Comments

@jku
Copy link
Member

jku commented May 21, 2021

ngclient type annotations are not complete: once mypy is in use (#1395), we should

  • enable mypy for ngclient
  • fix any annotation issues
@jku jku added the ngclient label May 21, 2021
@sechkova sechkova added the backlog Issues to address with priority for current development goals label May 26, 2021
@sechkova sechkova added this to the weeks22-23 milestone May 26, 2021
@joshuagl joshuagl removed this from the weeks22-23 milestone Jun 9, 2021
@sechkova sechkova added this to the weeks26-27 milestone Jun 23, 2021
@jku
Copy link
Member Author

jku commented Jun 30, 2021

I was planning to handle this but will push forward: this makes sense after both #1408 and #1455 are merged.

@joshuagl joshuagl assigned sechkova and unassigned jku Jul 7, 2021
@joshuagl joshuagl modified the milestones: weeks26-27, Sprint 4 Jul 7, 2021
@joshuagl joshuagl removed the backlog Issues to address with priority for current development goals label Jul 7, 2021
@sechkova sechkova mentioned this issue Jul 9, 2021
3 tasks
@sechkova
Copy link
Contributor

sechkova commented Jul 9, 2021

I had partial success when trying to enable mypy for ngclient: #1489

Two main issues remain:

  1. mypy cannot figure out our intentions to instantiate Signed subclasses
  2. mypy considers some values as potentially "None" at places where we think we have ensured that this is not possible (esp in trusted_metadata_set where we know the state but mypy doesn't)

see https://github.com/theupdateframework/tuf/pull/1489/checks?check_run_id=3028663485

The first issue could be resolved by #1457 or by adding type hints for all attributes missing in the base class (doesn't feel so great). While the second one probably requires case by case decisions.

@joshuagl joshuagl modified the milestones: Sprint 4, Sprint 5 Jul 28, 2021
@jku jku modified the milestones: Sprint 5, Sprint 6 Aug 18, 2021
@jku
Copy link
Member Author

jku commented Aug 18, 2021

this is no longer blocked by missing Metadata generics: I just merged that

@jku jku closed this as completed in #1489 Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants