Cu-86d2e0bc3: scheduled task hourly support#257
Merged
Conversation
📝 WalkthroughWalkthroughAdds a Changes
Sequence DiagramsequenceDiagram
actor User
participant Dialog as ScheduleEmailAlertDialog
participant Service as ScheduledJobService
participant BigQuery as BigQueryDatasource
User->>Dialog: choose "last_hour" and save
Dialog->>Service: create scheduled job (date_range='last_hour')
Service->>Service: _resolve_runtime_params() -> compute UTC now-1h to now, format params
Service->>BigQuery: execute_dynamic_query(params)
BigQuery->>BigQuery: for each parameter descriptor, if type=='timestamp' parse string -> UTC datetime
BigQuery-->>Service: query results (rows)
Service->>Service: if rows empty -> log and return
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
wavefront/server/plugins/datasource/datasource/bigquery/__init__.py (2)
115-123: Timestamp parsing is reasonable; consider narrowing the exception type.The fallback to the original value on parse failure is appropriate for robustness. The static analysis flags catching bare
Exception(BLE001), but this is acceptable here sincestrptimecan raiseValueErrorand the fallback behavior is safe. If you want to be more explicit:♻️ More specific exception handling
- except Exception: + except ValueError: params_to_execute[key] = value🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wavefront/server/plugins/datasource/datasource/bigquery/__init__.py` around lines 115 - 123, Narrow the broad exception handler in the timestamp parsing branch to only catch the expected ValueError from datetime.strptime: in the block that checks param_types.get(key) == 'timestamp' and parses value with datetime.strptime (the code that sets params_to_execute[key] to value_dt.replace(tzinfo=timezone.utc) or falls back to params_to_execute[key] = value), replace the bare except Exception with except ValueError so only parse errors are caught and other unexpected exceptions still surface.
96-101: Redundantisinstancecheck.On line 98,
isinstance(params, dict)is always true because line 95's list comprehension[params['name'] for params in query_params]would already fail if items weren't dicts. Consider simplifying:♻️ Simplified param_types extraction
param_types = { - params['name']: ( - params.get('type') if isinstance(params, dict) else None - ) + params['name']: params.get('type') for params in query_params }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wavefront/server/plugins/datasource/datasource/bigquery/__init__.py` around lines 96 - 101, The comprehension that builds param_types uses an unnecessary isinstance(params, dict) check because query_params items must be dicts (the earlier comprehension accessing params['name'] would have failed otherwise); simplify the comprehension for param_types by removing the isinstance guard and directly use params.get('type') (i.e., change the dict comprehension that assigns to param_types to rely on params.get('type') for each params in query_params).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@wavefront/server/plugins/datasource/datasource/bigquery/__init__.py`:
- Around line 115-123: Narrow the broad exception handler in the timestamp
parsing branch to only catch the expected ValueError from datetime.strptime: in
the block that checks param_types.get(key) == 'timestamp' and parses value with
datetime.strptime (the code that sets params_to_execute[key] to
value_dt.replace(tzinfo=timezone.utc) or falls back to params_to_execute[key] =
value), replace the bare except Exception with except ValueError so only parse
errors are caught and other unexpected exceptions still surface.
- Around line 96-101: The comprehension that builds param_types uses an
unnecessary isinstance(params, dict) check because query_params items must be
dicts (the earlier comprehension accessing params['name'] would have failed
otherwise); simplify the comprehension for param_types by removing the
isinstance guard and directly use params.get('type') (i.e., change the dict
comprehension that assigns to param_types to rely on params.get('type') for each
params in query_params).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3ee9277d-fd24-4b5c-bf1a-7ccf371a7068
📒 Files selected for processing (5)
wavefront/client/src/lib/utils.tswavefront/client/src/pages/apps/[appId]/datasources/ScheduleEmailAlertDialog.tsxwavefront/client/src/types/scheduled-job.tswavefront/server/apps/floware/floware/services/scheduled_job_service.pywavefront/server/plugins/datasource/datasource/bigquery/__init__.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
wavefront/server/apps/floware/floware/services/scheduled_job_service.py (1)
405-424: Dead code after early return.After the early return at line 415 when
rowsis empty, theif rows:check at line 417 is always true, making lines 423-424 unreachable.♻️ Suggested simplification
if len(rows) == 0: start_key = str(payload.get('start_date_param', 'start_date')) end_key = str(payload.get('end_date_param', 'end_date')) applied_start = params.get(start_key) if isinstance(params, dict) else None applied_end = params.get(end_key) if isinstance(params, dict) else None logger.info( f'No records in scheduled window for query_id={query_id}; ' f'applying range {applied_start}..{applied_end} (keys: {start_key}, {end_key}). ' 'Skipping email.' ) return - if rows: - fieldnames = list(rows[0].keys()) - for row in rows[1:]: - for k in row: - if k not in fieldnames: - fieldnames.append(k) - else: - fieldnames = [] + fieldnames = list(rows[0].keys()) + for row in rows[1:]: + for k in row: + if k not in fieldnames: + fieldnames.append(k)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wavefront/server/apps/floware/floware/services/scheduled_job_service.py` around lines 405 - 424, The code has an early return when len(rows) == 0, so the subsequent conditional "if rows: ... else: fieldnames = []" is dead/ redundant; remove the else branch and simplify by directly computing fieldnames = list(rows[0].keys()) and merging any additional keys from remaining rows (loop over rows[1:]) since at that point rows is guaranteed non-empty; update the block containing rows, fieldnames, and the for-loops (referenced symbols: rows, fieldnames, payload, params, query_id in scheduled_job_service.py) to remove the unreachable "else: fieldnames = []" branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@wavefront/server/apps/floware/floware/services/scheduled_job_service.py`:
- Around line 405-424: The code has an early return when len(rows) == 0, so the
subsequent conditional "if rows: ... else: fieldnames = []" is dead/ redundant;
remove the else branch and simplify by directly computing fieldnames =
list(rows[0].keys()) and merging any additional keys from remaining rows (loop
over rows[1:]) since at that point rows is guaranteed non-empty; update the
block containing rows, fieldnames, and the for-loops (referenced symbols: rows,
fieldnames, payload, params, query_id in scheduled_job_service.py) to remove the
unreachable "else: fieldnames = []" branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d05c4bb4-b5bd-4ecc-ada8-a67d72b8266f
📒 Files selected for processing (1)
wavefront/server/apps/floware/floware/services/scheduled_job_service.py
thomastomy5
pushed a commit
that referenced
this pull request
Apr 27, 2026
* fix(floware): for supporting scheduled jobs with hourly data query * Adding support for last hour * Dont send email if no records in db
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes