ref: fix types for monitor_consumer #83831
Conversation
github-actions
bot
added
the
Scope: Backend
| params["duration"] = ( | ||
| if params.get("duration") is not None: | ||
| # Duration is specified in seconds from the client, it is | ||
| # stored in the checkin model as milliseconds | ||
| int(params["duration"] * 1000) | ||
| if params.get("duration") is not None | ||
| else None | ||
| ) | ||
| params["duration"] = int(params["duration"] * 1000) |
There was a problem hiding this comment.
There's a small behavior change here right, since before it was setting params["duration"] to none even if it wasn't part of the dict. Now it will leave it not part of the dict.
not sure if this is actually important.
There was a problem hiding this comment.
there is yeah, but the types forbid None in this dict -- params are passed through a validator which normalizes it to None
afaict this still works since duration is only read from the validated params
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
|
PR reverted: 6ff0bb7 |
This reverts commit 6b69f76. Co-authored-by: IanWoodard <[email protected]>
| @@ -501,13 +508,10 @@ def _process_checkin(item: CheckinItem, txn: Transaction | Span): | |||
|
|
|||
| monitor_config = params.pop("monitor_config", None) | |||
|
|
|||
| params["duration"] = ( | |||
| if params.get("duration") is not None: | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
hmm I'm not so sure -- the error is before that line 🤔 this might be the schemas update I made? though afaict it should be fine with string there
There was a problem hiding this comment.
yeah this was the schemas bump instead and actually not this patch. suspect commits blamed this before it had even gone out
<!-- Describe your PR here. -->
This reverts commit 6b69f76. Co-authored-by: IanWoodard <[email protected]>
This reverts commit 6ff0bb7.
No description provided.