Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion backend/infrahub/core/schema/definitions/core/check.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
Attr(name="description", kind="Text", optional=True),
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),
Copy link
Contributor

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:

#!/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=10 in tests and fixtures with timeout=core_check_definition.timeout or 60, 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=10 occurrences)
    • 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.

Attr(name="parameters", kind="JSON", optional=True),
],
relationships=[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Copy link
Contributor

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

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:

  1. 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
  2. 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.

],
relationships=[
Rel(
Expand Down