-
Notifications
You must be signed in to change notification settings - Fork 35
Increase default timeout to 60 seconds for transforms. #7299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: stable
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,7 +29,7 @@ | |
| Attr(name="name", kind="Text", unique=True), | ||
| Attr(name="label", kind="Text", optional=True), | ||
| Attr(name="description", kind="Text", optional=True), | ||
| Attr(name="timeout", kind="Number", default_value=10), | ||
| Attr(name="timeout", kind="Number", default_value=60), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major 🧩 Analysis chainLGTM! Verify test expectations and documentation. The timeout increase from 10 to 60 seconds aligns with the PR objective to provide customers more time for artifact transforms. Run the following script to verify that tests and documentation don't hardcode the old timeout value: Additionally, consider:
🏁 Script executed: #!/bin/bash
# Description: Check for hardcoded references to the old 10-second timeout that may need updating.
# Search for test files that might assert or expect a 10-second timeout for transforms
echo "=== Checking test files for transform timeout references ==="
rg -n -C3 --type=py -g '*test*.py' 'timeout.*10|10.*timeout' backend/
# Search for documentation files mentioning the 10-second timeout
echo -e "\n=== Checking documentation for timeout references ==="
fd -e md -e rst -e txt . | xargs rg -n -C2 'transform.*timeout|timeout.*10'
# Search for other schema or config files that might reference transform timeout
echo -e "\n=== Checking for other transform timeout references ==="
rg -n -C3 --type=py 'transform.*timeout|timeout.*transform' backend/ | grep -v 'definitions/core/transform.py'Length of output: 18775 Update tests to reflect new default timeout Unit tests in backend/tests/unit/git/test_git_transform.py (lines 29, 54, 80, 102) and the default in backend/tests/unit/git/conftest.py:1248 still reference a 10-second timeout; update these to 60 seconds. 🤖 Prompt for AI Agents |
||
| ], | ||
| relationships=[ | ||
| Rel( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - Increase default timeout for transform/checks from 10 to 60 seconds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify that the timeout increase is intentional for check definitions.
The PR description focuses on "artifact transforms," but this change applies to
core_check_definition. While the technical change is valid, please confirm whether increasing the timeout for check definitions is intentional, or if this file was included unintentionally.Run the following script to verify the impact:
🏁 Script executed:
Length of output: 1482
Update tests and fixtures to use the new 60-second default timeout
timeout=10in tests and fixtures withtimeout=core_check_definition.timeoutor60, e.g. in:• backend/tests/helpers/test_worker.py:32
• backend/tests/unit/git/test_git_repository.py (lines asserting
modified_check0.timeout.value)• backend/tests/unit/git/test_git_transform.py (all
timeout=10occurrences)• backend/tests/fixtures/repos/car-dealership/.../transforms/car_spec_markdown.py:8
🤖 Prompt for AI Agents