Skip to content

fix: fill the missing data in the metrics response#5479

Merged
ilayda-cp merged 3 commits intomainfrom
WD-15452-prevent-count-reset
Nov 25, 2025
Merged

fix: fill the missing data in the metrics response#5479
ilayda-cp merged 3 commits intomainfrom
WD-15452-prevent-count-reset

Conversation

@ilayda-cp
Copy link
Copy Markdown
Contributor

@ilayda-cp ilayda-cp commented Nov 24, 2025

Done

Sometimes the metrics information that is provided from store API includes None values. This pr fillsout those missing data as follows:

  • take avg of before non-null and after non-null value
  • set the prev non-null val if its the last value
  • set the after non-null val if its the first value
  • set 0 if every thing is None

How to QA

Testing

  • This PR has tests
  • No testing required (explain why):

Issue / Card

Fixes https://warthogs.atlassian.net/browse/WD-15452

@webteam-app
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@steverydz steverydz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Copy link
Copy Markdown
Contributor

@edisile edisile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't QA because I don't have access to that snap, but code looks good

Comment thread webapp/metrics/helper.py
Comment on lines +176 to +178
while prev_idx >= 0 and prev_value is None:
prev_value = filled[prev_idx]
prev_idx -= 1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a case where the while is actually necessary? Since we're iterating items in order I think we can assume that the previous spot in the list is not None because it was either a valid number from the start or it has already been filled if it was None...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought not sure if changing it is worth it, I find it's nice that it's symmetrical to the next_value loop

@ilayda-cp ilayda-cp merged commit 036ca22 into main Nov 25, 2025
13 checks passed
@ilayda-cp ilayda-cp deleted the WD-15452-prevent-count-reset branch November 25, 2025 07:13
ilayda-cp added a commit that referenced this pull request Nov 27, 2025
ilayda-cp added a commit that referenced this pull request Nov 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants