Increase default timeout to 60 seconds for transforms. #7299
Conversation
WalkthroughUpdated schema defaults: the `timeout` attribute default was changed from 10 to 60 in two definitions — the public entity `core_transform` (backend/infrahub/core/schema/definitions/core/transform.py) and `core_check_definition` (backend/infrahub/core/schema/definitions/core/check.py). No other attributes, relationships, control flow, or error handling were modified. Estimated review effort: Medium. Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
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 |
github-actions
bot
added
the
group/backend
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/infrahub/core/schema/definitions/core/transform.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
backend/**/*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run backend tests with
pytestor viainvoketasks
Files:
backend/infrahub/core/schema/definitions/core/transform.py
**/*.py
📄 CodeRabbit inference engine (.github/instructions/python-docstring.instructions.md)
**/*.py: Use triple double quotes (""") for all Python docstrings
Write docstrings in Google-style format
Include a brief one-line description at the top of each docstring
Add a detailed description section when additional context is needed
Document function/method parameters under an Args/Parameters section without typing information
Include a Returns section describing the return value
Include a Raises section listing possible exceptions
Provide an Examples section demonstrating usage when helpful
**/*.py: Use type hints for all function parameters and return values in Python code
Prefer asynchronous implementations when feasible (use Async whenever possible)
Define asynchronous functions withasync def
Useawaitfor asynchronous calls
Use Pydantic models instead of standard library dataclasses
All Python functions must include a docstring
Use Ruff and MyPy for linting and type checkingUse ruff and mypy to validate and lint Python files
**/*.py: Use type hints for all Python function parameters and return values
Use async/await whenever possible in Python code
Useasync deffor asynchronous functions
Useawaitfor asynchronous calls within async functions
Use Pydantic models instead of dataclasses for data modeling
All Python functions must have docstrings
Docstrings must use triple quotes (""")
Follow Google-style docstring format
Docstrings should include, when applicable: brief description, detailed description, Args/Parameters (without types), Returns, Raises, and Examples
Validate and lint Python with ruff and mypy
Format Python code using the project’s formatter (invoke format)
Files:
backend/infrahub/core/schema/definitions/core/transform.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: E2E-testing-version-upgrade / From 1.3.0
- GitHub Check: E2E-testing-playwright
- GitHub Check: E2E-testing-invoke-demo-start
- GitHub Check: backend-benchmark
- GitHub Check: backend-docker-integration
- GitHub Check: backend-tests-functional
- GitHub Check: backend-tests-integration
- GitHub Check: validate-generated-documentation
- GitHub Check: backend-tests-unit
- GitHub Check: Cloudflare Pages
| 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.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
LGTM! 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:
- Whether existing transform instances with the old default (10 seconds) will automatically pick up the new default (60 seconds) or if a migration is needed
- If release notes or upgrade documentation should mention this change for existing deployments
🏁 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
In backend/infrahub/core/schema/definitions/core/transform.py around line 32 the
default timeout was changed to 60s; update the unit tests that still expect 10s:
change the timeout assertions/expected values at
backend/tests/unit/git/test_git_transform.py lines 29, 54, 80, and 102 from 10
to 60, and update the default timeout value in
backend/tests/unit/git/conftest.py at line 1248 from 10 to 60 (and any related
fixture/comment values) so tests match the new default.
CodSpeed Performance ReportMerging #7299 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/infrahub/core/schema/definitions/core/check.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
backend/**/*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run backend tests with
pytestor viainvoketasks
Files:
backend/infrahub/core/schema/definitions/core/check.py
**/*.py
📄 CodeRabbit inference engine (.github/instructions/python-docstring.instructions.md)
**/*.py: Use triple double quotes (""") for all Python docstrings
Write docstrings in Google-style format
Include a brief one-line description at the top of each docstring
Add a detailed description section when additional context is needed
Document function/method parameters under an Args/Parameters section without typing information
Include a Returns section describing the return value
Include a Raises section listing possible exceptions
Provide an Examples section demonstrating usage when helpful
**/*.py: Use type hints for all function parameters and return values in Python code
Prefer asynchronous implementations when feasible (use Async whenever possible)
Define asynchronous functions withasync def
Useawaitfor asynchronous calls
Use Pydantic models instead of standard library dataclasses
All Python functions must include a docstring
Use Ruff and MyPy for linting and type checkingUse ruff and mypy to validate and lint Python files
**/*.py: Use type hints for all Python function parameters and return values
Use async/await whenever possible in Python code
Useasync deffor asynchronous functions
Useawaitfor asynchronous calls within async functions
Use Pydantic models instead of dataclasses for data modeling
All Python functions must have docstrings
Docstrings must use triple quotes (""")
Follow Google-style docstring format
Docstrings should include, when applicable: brief description, detailed description, Args/Parameters (without types), Returns, Raises, and Examples
Validate and lint Python with ruff and mypy
Format Python code using the project’s formatter (invoke format)
Files:
backend/infrahub/core/schema/definitions/core/check.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: E2E-testing-version-upgrade / From 1.3.0
- GitHub Check: E2E-testing-playwright
- GitHub Check: backend-benchmark
- GitHub Check: E2E-testing-invoke-demo-start
- GitHub Check: backend-tests-functional
- GitHub Check: backend-docker-integration
- GitHub Check: validate-generated-documentation
- GitHub Check: backend-validate-generated
- GitHub Check: backend-tests-unit
- GitHub Check: backend-tests-integration
- GitHub Check: Cloudflare Pages
| Attr(name="file_path", kind="Text"), | ||
| Attr(name="class_name", kind="Text"), | ||
| Attr(name="timeout", kind="Number", default_value=10), | ||
| Attr(name="timeout", kind="Number", default_value=60), |
There was a problem hiding this comment.
🛠️ 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:
#!/bin/bash
# Description: Check for existing references to check definition timeouts and related tests
echo "=== Searching for check definition timeout references ==="
rg -n "CheckDefinition" -A 5 -B 5 | rg -i "timeout"
echo -e "\n=== Searching for hardcoded 10-second timeout expectations in tests ==="
rg -n --type=py "timeout.*=.*10" backend/tests/
echo -e "\n=== Searching for documentation mentioning check timeouts ==="
rg -n -i "check.*timeout|timeout.*check" --type=md
echo -e "\n=== Checking transform.py for comparison ==="
cat backend/infrahub/core/schema/definitions/core/transform.py | rg -n "timeout.*default_value"Length of output: 1482
Update tests and fixtures to use the new 60-second default timeout
- Replace hardcoded
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 assertingmodified_check0.timeout.value)
• backend/tests/unit/git/test_git_transform.py (alltimeout=10occurrences)
• backend/tests/fixtures/repos/car-dealership/.../transforms/car_spec_markdown.py:8
🤖 Prompt for AI Agents
In backend/infrahub/core/schema/definitions/core/check.py around line 32, the
default timeout was changed to 60 but tests and fixtures still use hardcoded
timeout=10; update the referenced tests and fixtures to use the new default by
replacing timeout=10 with either timeout=core_check_definition.timeout
(importing core_check_definition where appropriate) or timeout=60. Specifically
modify backend/tests/helpers/test_worker.py (line ~32),
backend/tests/unit/git/test_git_repository.py assertions that check
modified_check0.timeout.value, all timeout=10 occurrences in
backend/tests/unit/git/test_git_transform.py, and
backend/tests/fixtures/repos/car-dealership/.../transforms/car_spec_markdown.py:8
so they reference the new default instead of the literal 10. Ensure imports are
added if using core_check_definition and update expected assertion values to 60
where applicable.
|
I'm fine with this change and I think we can merge it. Having said that we also have other issues to refactor this timeout alltogether. I don't remember the issue id now, but the idea would be to mark all of these timeouts as optional and default to being optional. That would mean that if a user hasn't set a timeout we'd instead just use the timeoout of the SDK used within the prefect runner. One comment though I don't think the change for the Python Transform is enough, even if we update the default is ok for now if I don't remember incorrectly the change needs to happen here too: https://github.com/opsmill/infrahub-sdk-python/blob/v1.14.0/infrahub_sdk/transforms.py#L20 Also we should have a newsfragment. |
I will open a PR today to increase those timeouts as well. |
There was a problem hiding this comment.
LGTM, however I think we should wait to merge this until the code from opsmill/infrahub-sdk-python#563 also lives within the stable branch of Infrahub to avoid confusion.
I think we can target this for Infrahub 1.5.0. The alternative would be if we released a 1.14.1 version of the SDK and brought that commit into stable. The problem would be if we wanted to release a new 1.4.x version of Infrahub that didn't include that SDK commit.
2 participants
We believe it may be good to increase the timeout of transforms from 10 seconds to 60 seconds which should give customers enough room for most artifact transforms.
Summary by CodeRabbit