Skip to content

Conversation

@Nabil-Salah
Copy link
Contributor

Description

Describe the changes introduced by this PR and what does it affect

Changes

List of changes this PR includes

Related Issues

Checklist

  • Tests included
  • Build pass
  • Documentation
  • Code format and docstring

Signed-off-by: nabil salah <nabil.salah203@gmail.com>
}

func (p *PostgresDatabase) calculateUptimeScore(ctx context.Context, healthReport types.HealthReport) float64 {
const thirtyDaysInSeconds = 30 * 24 * 60
Copy link
Member

Choose a reason for hiding this comment

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

thirtyDaysInSeconds Should be 30 * 24 * 60 * 60 ?

endTime := thirtyDaysAgo + 60

err = p.gormDB.WithContext(ctx).Table("health_report").
Where("node_twin_id = ? AND updated_at BETWEEN ? AND ?", healthReport.NodeTwinId, startTime, endTime).
Copy link
Member

Choose a reason for hiding this comment

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

I think this is a critical flaw for this PR

This assumes historic rows to subtract the value falling out of the 30-day window, but health_report currently stores only the latest state per node (see UpsertNodeHealth and also HealthReport struct, where NodeTwinId has a unique tag), and there’s no history table.

As a result, Score won’t reflect a true x-day window. Queries for “old” rows will not find meaningful data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants