fix: prevent invalid block trigger configurations from being accepted #339
+185
−35
Conversation
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
Member
chrisli30
commented
Jun 20, 2025
chrisli30
commented
- fix: prevent invalid block trigger configurations from being accepted
- Make TestOperatorConnectionStabilization tests not FAIL
- Fix Task.Validate() to properly check for nil configs (was allowing nil configs to pass validation) - Add ValidateWithError() method for detailed validation error messages - Add comprehensive validation for all trigger types (block, cron, fixed time, event) - Add DetectAndHandleInvalidTasks() method to clean up existing invalid tasks on startup - Add test for nil block trigger config validation - Improve error messages to be more specific about validation failures This prevents tasks with invalid configurations (like interval=0 or nil configs) from being created in the first place, and provides a cleanup mechanism for any existing invalid tasks.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue by preventing invalid block trigger configurations from being accepted and enhances test stability. Key changes include:
- Refactoring task validation to return detailed error messages.
- Updating integration test log messages to clarify expected timeouts.
- Introducing engine logic to detect and mark invalid tasks as failed.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| model/task.go | Refactored validation logic to provide detailed error messages. |
| integration_test/ticker_context_test.go | Updated log messages to provide clearer information during timeouts. |
| integration_test/orphaned_task_reclamation_test.go | Updated log messages for improved clarity on connection cleanup expectations. |
| integration_test/operator_reconnection_test.go | Updated log messages to denote normal cleanup timing. |
| core/taskengine/engine_crud_test.go | Added a new test to assert nil block trigger config is correctly rejected. |
| core/taskengine/engine.go | Added logic to detect and handle invalid task configurations at startup. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.