Conversation
haraldsvik
left a comment
haraldsvik
left a comment
There was a problem hiding this comment.
Great job! Feels good to have improved typing! 🦄
just have a few comments/questions
| update_type: Union[str, None] | ||
| data_structure_updates: List[DataStructureUpdate] | ||
| update_type: str | None | ||
| data_structure_updates: list[DataStructureUpdate] |
There was a problem hiding this comment.
I see you updated List -> to list a few places, but we still use it many places. I know its a chore to update all the references 😅 and it doesn't really affect anything except consistency.
Super nitpicky sorry! 🙇 It just became alot more visible when all the rest of the typing improved.
There was a problem hiding this comment.
This is a housekeeping PR so if there is a place to nitpick it's here 👍🏻 I'll take a walk through the repo and catch the inconsistencies 💯
| operation="PATCH_METADATA", | ||
| description=description, | ||
| releaseStatus="DRAFT", | ||
| release_status="DRAFT", |
There was a problem hiding this comment.
I see there is alot of references for both releaseStatus and release_status
release_status is for "job-service" and "releaseStatus" is for datastore right? Its a little confusing when its been a while since I've looked at those part. I first thought i was missing some conversions somewhere
There was a problem hiding this comment.
I'll take a look around, but pyright wants us to use the actual field names rather than the camelcased alias in the python code (which is way prettier) so I have fixed all these lints
|
haraldsvik
left a comment
haraldsvik
left a comment
There was a problem hiding this comment.
Looks good! lots of great changes 💯
| queued_worker_jobs: List[Job] | ||
| built_jobs: List[Job] | ||
| queued_manager_jobs: List[Job] | ||
| queued_worker_jobs: list[Job] |
2 participants
Better Typing
Notes
Had to break out a bunch of
# type: ignoresince we are overriding a bunch of pydantic init methods to read from the file system. This should be fixed for a bunch of other reasons. But it should be a part of a planned refactor and not a PR for improved typing.