ref(typing): Typing for the low priority symbolication queue #29017
Conversation
| @@ -106,6 +110,7 @@ def add_project_to_lpq(self, project_id: int) -> bool: | |||
| """ | |||
| raise NotImplementedError | |||
|
|
|||
| # TODO(flub): See if this can be removed | |||
There was a problem hiding this comment.
arguably i shouldn't have left this comment here. but it seems you should just be able to use the remove_projects_from_lpq() call instead, I don't think there's anything that this signature gives you that you can't do from the other one.
| for project_id in deleted_projects: | ||
| # TODO: add metrics! | ||
| logger.warning("Moved project out of symbolicator's low priority queue: %s", project_id) |
There was a problem hiding this comment.
This is the biggest logical change: we no longer try to figure out which projects may or may not have been removed (because the API doesn't allow us). Instead we rely on the fact the API will succeed or will raise an exception. So if we didn't die with an exception then this succeeded and all these projects were removed from the LPQ.
In theory a project could already have been removed before so it wasn't actually removed just now, but this really doesn't matter. And in any case if that happens our tasks are not keeping up with real time.
fc3e29e to
1644da0
Compare
888f333 to
fa8ea92
Compare
e618b74 to
b461030
Compare
17cbee5 to
c33a984
Compare
There was a problem hiding this comment.
rebased off of #28714 as a shared parent branch just got merged into master.
also added in a commit to remove remove_project_from_lpq as after a discussion with @flub it turns out that this method just causes confusion because there's only a 1 character difference between that and its plural/set-equivalent operation. API symmetry isn't really doing us any favours in that situation; maybe if python was more properly strongly-typed then we could wave something like this off, but i don't think it makes sense in this situation.
|
merging this in after #28714 so the commit tied to this isn't squashed in together with #28714's changes. this is mostly for personal/future reference in case somebody like myself decides to look back at an existing commit that adds in typing as opposed to having to find all of the different spots that need to be updated from scratch. |
The most important change is the src/sentry/processing/__init__.py which makes mypy recognise this package correctly. From that more type checking was working and types needed to be fixed up.
4ddf914 to
715970c
Compare
The most important change is the src/sentry/processing/init.py
which makes mypy recognise this package correctly. From that more
type checking was working and types needed to be fixed up.