fix(workflows): Fix empty results in Workflows query type#526
Conversation
|
|
|
|
zoltanbedi
left a comment
zoltanbedi
left a comment
There was a problem hiding this comment.
Thanks for creating this PR @idan-starkware. I tried it out and it worked as expected. Please create a changeset for this and also update the docs in docs/sources/query/_index.md.
pkg/github/workflows.go
Outdated
|
|
||
| // If time field is None, return all workflows without filtering | ||
| if opts.TimeField == models.WorkflowTimeFieldNone { | ||
| backend.Logger.Debug("GetWorkflows", "time_field_none", "returning_all_workflows") |
There was a problem hiding this comment.
We can figure it out from the debug log above that the time field is none.
| backend.Logger.Debug("GetWorkflows", "time_field_none", "returning_all_workflows") |
pkg/github/workflows.go
Outdated
| shouldInclude = !createdAtTime.Before(timeRange.From) && !createdAtTime.After(timeRange.To) | ||
| if !shouldInclude { | ||
| excludedCount++ | ||
| backend.Logger.Debug("keepWorkflowsInTimeRange", "workflow_excluded", *workflow.Name, "createdAt", createdAtTime, "timeRange", fmt.Sprintf("%v to %v", timeRange.From, timeRange.To)) |
There was a problem hiding this comment.
Please remove this debug log. The summary debug log is enough.
pkg/github/workflows.go
Outdated
| shouldInclude = !updatedAtTime.Before(timeRange.From) && !updatedAtTime.After(timeRange.To) | ||
| if !shouldInclude { | ||
| excludedCount++ | ||
| backend.Logger.Debug("keepWorkflowsInTimeRange", "workflow_excluded", *workflow.Name, "updatedAt", updatedAtTime, "timeRange", fmt.Sprintf("%v to %v", timeRange.From, timeRange.To)) |
There was a problem hiding this comment.
Please remove this debug log. The summary debug log is enough.
idan-starkware
force-pushed
the
fix/workflows-empty-results-503
branch
from
18f2663 to
df670e2
Compare
There was a problem hiding this comment.
@idan-starkware the debug logs made it back that @jcolladokuri removed. Please remove them.
Also please merge from main as I fixed the zizmor issues.
Fixes grafana#503 - Add nil check for CreatedAt/UpdatedAt timestamps to prevent panics - Add 'None' option to Time Field dropdown (default) to return all workflows - Improve time filtering logic with better nil handling - Add debug logging for troubleshooting
- Add 'None' option to Time Field documentation - Update sample queries to show both 'None' and 'CreatedAt' options - Add changeset for the fix
- Remove time_field_none debug log (redundant with timeField in main log) - Remove per-workflow exclusion logs (summary log is sufficient)
Remove all debug logging statements that were previously removed by jcolladokuri
idan-starkware
force-pushed
the
fix/workflows-empty-results-503
branch
from
628678b to
2386c3d
Compare
3 participants
Fixes #503
Testing