[myakove]

PR Type

Enhancement

---

Description

• Simplify source provider configuration from two CLI args to single unified arg

  • Replace --tc=source_provider_type and --tc=source_provider_version with --tc=source_provider
  • Provider key directly maps to .providers.json entry (e.g., vsphere-8.0.1)

• Extract provider loading logic into reusable load_source_providers() utility function

• Update test markers to resolve provider type dynamically from .providers.json

• Update documentation and examples across README and copyoffload guides

• Clean up unused imports and normalize whitespace in test files

---

Diagram Walkthrough

flowchart LR
  A["Two CLI Args<br/>source_provider_type<br/>source_provider_version"] -->|Consolidate| B["Single CLI Arg<br/>source_provider"]
  B -->|Maps to| C[".providers.json<br/>Key"]
  C -->|Loaded by| D["load_source_providers()"]
  D -->|Used in| E["Test Fixtures<br/>& Markers"]

Loading

File Walkthrough

	Relevant files
Enhancement	conftest.py Consolidate provider config and use utility function          conftest.py Remove MissingProvidersFileError import and replace inline provider loading with load_source_providers() utility Update required config from two separate keys to single source_provider key Simplify source_provider_data fixture to directly lookup provider using single config key Update test name generation to use single source_provider instead of separate type and version Add comprehensive docstring to source_provider_data fixture +13/-17  test_copyoffload_migration.py Update imports and provider type resolution                            tests/test_copyoffload_migration.py Remove unused TimeoutSampler import Add load_source_providers() import and create module-level _SOURCE_PROVIDER_TYPE variable Update skipif markers to resolve provider type from loaded providers instead of direct config access Fix import ordering to follow alphabetical convention Add error message template constant for migration validation +12/-5    test_mtv_warm_migration.py Update provider type resolution from config                            tests/test_mtv_warm_migration.py Add load_source_providers() import Replace direct py_config.get("source_provider_type") with dynamic lookup from loaded providers Update module-level SOURCE_PROVIDER_TYPE variable to _SOURCE_PROVIDER_TYPE for consistency Update all references to use new provider type resolution method +5/-5      utils.py Add load_source_providers utility function                              utilities/utils.py Add json import for provider file parsing Add MissingProvidersFileError import from exceptions Create new load_source_providers() function to load and return provider configurations from .providers.json Function includes error handling for missing provider file and comprehensive docstring +19/-0
Documentation	README.md Update documentation with simplified provider config          README.md Update all CLI examples to use single --tc=source_provider:vsphere-8.0.1 instead of separate type and version args Clarify that provider key from .providers.json is used directly in CLI configuration Simplify documentation explaining provider key format and usage Update Kubernetes Job YAML examples with consolidated provider argument Update placeholder descriptions to reference provider key instead of separate type/version +16/-24  how-to-run-copyoffload-tests.md Simplify provider key documentation                                            docs/copyoffload/how-to-run-copyoffload-tests.md Simplify provider key format explanation to clarify direct mapping from .providers.json Remove complex explanation about test loader key construction Update Kubernetes Job YAML example to use single --tc=source_provider argument Update customization notes to reference provider key instead of separate version +5/-14

---

Summary by CodeRabbit

• Documentation

  • Unified provider configuration guidance: use a single source_provider key in CLI examples (e.g., --tc=source_provider:vsphere-8.0.1) and updated docs/examples accordingly.

• Bug Fixes

  • Improved error messaging when the providers configuration file is missing or empty.

• Chores

  • Updated test and configuration setup to work with the unified provider key approach.

[coderabbitai]

Walkthrough

Consolidates separate provider_type/provider_version into a single source_provider key (e.g., vsphere-8.0.1), updates docs, pytest fixtures, and tests to use the new key, adds load_source_providers() utility, and refines a providers-related exception message.

Changes

Cohort / File(s)	Summary
Documentation README.md, docs/copyoffload/how-to-run-copyoffload-tests.md	Replace separate provider_type/provider_version guidance and examples with the unified provider key (e.g., --tc=source_provider:vsphere-8.0.3.00400) and update explanatory text and placeholders.
Pytest Fixture Configuration conftest.py	Fixture source_provider_data signature updated with type annotations and docstring; lookup now uses py_config["source_provider"]. source_providers fixture now returns load_source_providers(). Test naming and collection logic adjusted to the new key; added defensive collection short-circuit in pytest_exception_interact.
Test Files tests/test_copyoffload_migration.py, tests/test_mtv_warm_migration.py	Added load_source_providers import and introduced _SOURCE_PROVIDER_TYPE derived from loaded providers via py_config["source_provider"]. Updated skips/conditionals and message references to use _SOURCE_PROVIDER_TYPE.
Utilities utilities/utils.py	Added load_source_providers() — reads .providers.json and returns dict[str, dict[str, Any]]; returns empty dict if file absent. Includes docstring and JSON import.
Exceptions exceptions/exceptions.py	MissingProvidersFileError now defines __init__ that sets the message "'.providers.json' file is missing or empty" via super().__init__.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: consolidating two separate CLI arguments (--tc=source_provider_type and --tc=source_provider_version) into a single unified argument (--tc=source_provider), which is the primary objective across all modified files.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/issue-292-simplify-source-provider

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[redhat-qe-bot1]

Report bugs in Issues

Welcome! 🎉

This pull request will be automatically processed with the following features:

🔄 Automatic Actions

• Reviewer Assignment: Reviewers are automatically assigned based on the OWNERS file in the repository root
• Size Labeling: PR size labels (XS, S, M, L, XL, XXL) are automatically applied based on changes
• Issue Creation: Disabled for this repository
• Branch Labeling: Branch-specific labels are applied to track the target branch
• Auto-verification: Auto-verified users have their PRs automatically marked as verified
• Labels: All label categories are enabled (default configuration)

📋 Available Commands

PR Status Management

• /wip - Mark PR as work in progress (adds WIP: prefix to title)
• /wip cancel - Remove work in progress status
• /hold - Block PR merging (approvers only)
• /hold cancel - Unblock PR merging
• /verified - Mark PR as verified
• /verified cancel - Remove verification status
• /reprocess - Trigger complete PR workflow reprocessing (useful if webhook failed or configuration changed)
• /regenerate-welcome - Regenerate this welcome message

Review & Approval

• /lgtm - Approve changes (looks good to me)
• /approve - Approve PR (approvers only)
• /automerge - Enable automatic merging when all requirements are met (maintainers and approvers only)
• /assign-reviewers - Assign reviewers based on OWNERS file
• /assign-reviewer @username - Assign specific reviewer
• /check-can-merge - Check if PR meets merge requirements

Testing & Validation

• /retest tox - Run Python test suite with tox
• /retest build-container - Rebuild and test container image
• /retest all - Run all available tests

Container Operations

• /build-and-push-container - Build and push container image (tagged with PR number)
  • Supports additional build arguments: /build-and-push-container --build-arg KEY=value

Cherry-pick Operations

• /cherry-pick <branch> - Schedule cherry-pick to target branch when PR is merged
  • Multiple branches: /cherry-pick branch1 branch2 branch3

Label Management

• /<label-name> - Add a label to the PR
• /<label-name> cancel - Remove a label from the PR

✅ Merge Requirements

This PR will be automatically approved when the following conditions are met:

1. Approval: /approve from at least one approver
2. LGTM Count: Minimum 0 /lgtm from reviewers
3. Status Checks: All required status checks must pass
4. No Blockers: No WIP, hold, conflict labels
5. Verified: PR must be marked as verified (if verification is enabled)

📊 Review Process

Approvers and Reviewers

Approvers:

• myakove

Reviewers:

• krcmarik
• myakove

Available Labels

• hold
• verified
• wip
• lgtm
• approve
• automerge

💡 Tips

• WIP Status: Use /wip when your PR is not ready for review
• Verification: The verified label is automatically removed on each new commit
• Cherry-picking: Cherry-pick labels are processed when the PR is merged
• Container Builds: Container images are automatically tagged with the PR number
• Permission Levels: Some commands require approver permissions
• Auto-verified Users: Certain users have automatic verification and merge privileges

For more information, please refer to the project documentation or contact the maintainers.

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Type Error The new load_source_providers() return type annotation references Any, but the diff does not show Any being imported in this module. If annotations are evaluated at runtime (no postponed evaluation), this can raise a NameError on import. Ensure Any is available (import it or enable postponed evaluation). def load_source_providers() -> dict[str, dict[str, Any]]: """Load source providers from .providers.json. Returns: dict[str, dict[str, Any]]: Provider configurations keyed by provider name. Raises: MissingProvidersFileError: If .providers.json doesn't exist. """ providers_file = Path(".providers.json") if not providers_file.exists(): raise MissingProvidersFileError(f"{providers_file} file is missing") with open(providers_file, "r") as fd: return json.load(fd) Import-time IO _SOURCE_PROVIDER_TYPE is computed at module import time by calling load_source_providers(), which reads .providers.json and may raise if missing. This can break test collection (including --collectonly) and prevents skip markers from being evaluated gracefully. Consider deferring provider loading to fixtures/hooks or guarding missing file/config so collection can proceed with a clear skip/error. wait_for_migration_complate, ) from utilities.naming import sanitize_kubernetes_name from utilities.post_migration import check_vms from utilities.resources import create_and_store_resource from utilities.ssh_utils import SSHConnectionManager from utilities.utils import load_source_providers LOGGER = get_logger(__name__) _SOURCE_PROVIDER_TYPE = load_source_providers().get(py_config.get("source_provider", ""), {}).get("type") # Error message template for simultaneous migration validation EARLY_COMPLETION_MSG = ( "Plan {plan_num} reached {completed_status} before both plans were executing simultaneously. " "Other plan status: {other_status}" ) Missing Validation source_provider_data indexes source_providers[py_config["source_provider"]] directly, which will raise KeyError if the config key is missing/typoed or not present in .providers.json. Add explicit validation with a helpful error message listing available provider keys to reduce triage time. def source_provider_data(source_providers: dict[str, dict[str, Any]], fixture_store: dict[str, Any]) -> dict[str, Any]: """Resolve source provider configuration from .providers.json. Args: source_providers (dict[str, dict[str, Any]]): All provider configurations from .providers.json. fixture_store (dict[str, Any]): Session fixture store for teardown tracking. Returns: dict[str, Any]: The resolved source provider configuration dict. """ _source_provider = source_providers[py_config["source_provider"]] fixture_store["source_provider_data"] = _source_provider return _source_provider

---

💡 Tool usage guide:

---

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

• When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

[qodo-code-review]

PR Code Suggestions ✨

Latest suggestions up to e315581

Category	Suggestion	Impact
Possible bug	Fail fast on missing config Modify load_source_providers to raise an exception if the .providers.json file is missing, empty, or contains invalid JSON, instead of silently returning an empty dictionary. utilities/utils.py [41-55] def load_source_providers() -> dict[str, dict[str, Any]]: """Load source providers from .providers.json. Returns: dict[str, dict[str, Any]]: Provider configurations keyed by provider name. + + Raises: + MissingProvidersFileError: If `.providers.json` is missing or empty. + ValueError: If `.providers.json` is not valid JSON. """ providers_file = Path(".providers.json") - if not providers_file.exists(): - return {} + if not providers_file.exists() or providers_file.stat().st_size == 0: + raise MissingProvidersFileError() - with open(providers_file) as fd: - content = fd.read() - if not content.strip(): - return {} - return json.loads(content) + try: + with open(providers_file, encoding="utf-8") as fd: + providers: dict[str, dict[str, Any]] = json.load(fd) + except json.JSONDecodeError as exc: + raise ValueError(f"Invalid JSON in '{providers_file}': {exc}") from exc + if not providers: + raise MissingProvidersFileError() + + return providers + Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that returning an empty dictionary for a missing or invalid config file can cause silent downstream failures, and proposes a much more robust "fail-fast" approach by raising exceptions.	Medium
	Skip when provider type unknown In tests/test_mtv_warm_migration.py, add a pytest.mark.skipif to skip tests if _SOURCE_PROVIDER_TYPE is None, preventing execution when the provider type is unknown. tests/test_mtv_warm_migration.py [18-27] _SOURCE_PROVIDER_TYPE = load_source_providers().get(py_config.get("source_provider", ""), {}).get("type") - pytestmark = [ + pytest.mark.skipif( + _SOURCE_PROVIDER_TYPE is None, + reason="Cannot determine source provider type (missing/invalid '.providers.json' entry).", + ), pytest.mark.skipif( _SOURCE_PROVIDER_TYPE in (Provider.ProviderType.OPENSTACK, Provider.ProviderType.OPENSHIFT, Provider.ProviderType.OVA), reason=f"{_SOURCE_PROVIDER_TYPE} warm migration is not supported.", ), ] Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion improves test robustness by explicitly handling cases where the provider type cannot be determined, preventing tests from running with an invalid configuration.	Medium
More

---

Previous suggestions

✅ Suggestions up to commit 46e6b63

Category	Suggestion	Impact
Possible bug	✅ Validate and fail fast on config Suggestion Impact: Updated load_source_providers to explicitly detect an empty .providers.json (by reading and checking stripped content) before attempting JSON parsing, but it still returns {} rather than raising, and it does not add exception handling for invalid JSON. code diff: @@ -49,7 +49,10 @@ return {} with open(providers_file) as fd: - return json.load(fd) + content = fd.read() + if not content.strip(): + return {} + return json.loads(content) Improve the load_source_providers function to fail fast by raising an exception if the .providers.json file is missing, empty, or contains invalid JSON, instead of returning an empty dictionary. This prevents silent configuration errors. utilities/utils.py [41-52] def load_source_providers() -> dict[str, dict[str, Any]]: """Load source providers from .providers.json. Returns: dict[str, dict[str, Any]]: Provider configurations keyed by provider name. """ providers_file = Path(".providers.json") if not providers_file.exists(): - return {} + raise MissingProvidersFileError() - with open(providers_file) as fd: - return json.load(fd) + try: + with open(providers_file, encoding="utf-8") as fd: + data = json.load(fd) + except json.JSONDecodeError as ex: + raise ValueError(f"Invalid JSON in '{providers_file}': {ex}") from ex + if not isinstance(data, dict) or not data: + raise MissingProvidersFileError() + + return data + Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that returning an empty dictionary from load_source_providers can lead to silent failures and makes the new function more robust by adding explicit error handling for missing, empty, or invalid configuration files.	Medium
Performance	Cache provider config loading Optimize the load_source_providers function by caching its result using @functools.lru_cache. This avoids repeated file reading and JSON parsing, improving performance during the test session. utilities/utils.py [41-52] +@functools.lru_cache(maxsize=1) def load_source_providers() -> dict[str, dict[str, Any]]: """Load source providers from .providers.json. Returns: dict[str, dict[str, Any]]: Provider configurations keyed by provider name. """ providers_file = Path(".providers.json") if not providers_file.exists(): return {} - with open(providers_file) as fd: + with open(providers_file, encoding="utf-8") as fd: return json.load(fd) Suggestion importance[1-10]: 5 __ Why: This is a valid performance optimization. Caching the result of load_source_providers with lru_cache is a good practice to avoid redundant file I/O, as the function is called in multiple places during test setup.	Low

✅ Suggestions up to commit 5ff9eae

Category	Suggestion	Impact
best practice	Use project-root path for providers file Modify the path resolution for .providers.json to be relative to the project root, ensuring consistent file loading regardless of the current working directory. utilities/utils.py [51-53] -providers_file = Path(".providers.json") +providers_file = Path(__file__).resolve().parent.parent / ".providers.json" if not providers_file.exists(): raise MissingProvidersFileError(f"{providers_file} file is missing") Suggestion importance[1-10]: 7 __ Why: This suggestion correctly points out that using a relative path can be brittle and proposes a more robust solution by anchoring the file path to the script's location, making test execution more reliable regardless of the working directory.	Medium
	Create a fixture for provider type Replace the duplicated, module-level _SOURCE_PROVIDER_TYPE logic with a dedicated pytest fixture to improve readability and reduce code duplication. tests/test_copyoffload_migration.py [46] -_SOURCE_PROVIDER_TYPE = load_source_providers().get(py_config.get("source_provider", ""), {}).get("type") +@pytest.fixture(scope="session") +def source_provider_type(source_provider_data: dict[str, Any]) -> str | None: + """Get the 'type' of the source provider.""" + return source_provider_data.get("type") Suggestion importance[1-10]: 6 __ Why: This is a good refactoring suggestion that follows pytest best practices by replacing a module-level variable with a reusable fixture, improving code structure and maintainability.	Low
Possible bug	✅ Validate existence of provider key Suggestion Impact: The fixture now validates that providers data is loaded and that the requested source provider key exists; it raises a more user-friendly error (ValueError with available keys, plus a custom error when providers are missing) instead of an unhandled KeyError. code diff: - _source_provider = source_providers[py_config["source_provider"]] + if not source_providers: + raise MissingProvidersFileError() + + requested_provider = py_config["source_provider"] + if requested_provider not in source_providers: + raise ValueError( + f"Source provider '{requested_provider}' not found in '.providers.json'. " + f"Available providers: {sorted(source_providers.keys())}" + ) + + _source_provider = source_providers[requested_provider] fixture_store["source_provider_data"] = _source_provider Add a check in the source_provider_data fixture to validate that the provider key from the configuration exists in the loaded providers, raising a ValueError if not found. conftest.py [570-572] -_source_provider = source_providers[py_config["source_provider"]] +key = py_config["source_provider"] +if key not in source_providers: + raise ValueError(f"Source provider '{key}' not found in .providers.json") +_source_provider = source_providers[key] fixture_store["source_provider_data"] = _source_provider return _source_provider Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that a missing provider key will cause an unhandled KeyError and proposes adding an explicit check to raise a ValueError with a more user-friendly message, which significantly improves error handling.	Medium
Performance	Cache providers loading function Apply @functools.lru_cache to the load_source_providers function to cache the contents of .providers.json, avoiding redundant file I/O and improving performance. utilities/utils.py [42-56] +@functools.lru_cache(maxsize=1) def load_source_providers() -> dict[str, dict[str, Any]]: """Load source providers from .providers.json. Returns: dict[str, dict[str, Any]]: Provider configurations keyed by provider name. Raises: MissingProvidersFileError: If .providers.json doesn't exist. """ providers_file = Path(".providers.json") if not providers_file.exists(): raise MissingProvidersFileError(f"{providers_file} file is missing") with open(providers_file, "r") as fd: return json.load(fd) Suggestion importance[1-10]: 5 __ Why: The suggestion offers a valid performance improvement by caching the provider data, which is beneficial as the function is called in multiple test files at import time.	Low

[coderabbitai]

Actionable comments posted: 4

🤖 Fix all issues with AI agents

In `@conftest.py`:
- Around line 559-572: The fixture source_provider_data currently does a direct
dict lookup using py_config["source_provider"], which raises an unhelpful
KeyError on mismatch; change it to catch the missing-key case and raise a
ValueError with a clear diagnostic that includes the requested key and the list
of available keys (use source_providers.keys() or check with
source_providers.get), ensure fixture_store is only set when a valid provider is
found, and update the docstring to include a Raises section documenting the
ValueError on invalid/missing provider.

In `@tests/test_copyoffload_migration.py`:
- Line 46: The test is silently defaulting source_provider; replace the safe
.get default with direct access so missing config fails fast: change the
assignment for _SOURCE_PROVIDER_TYPE to read py_config['source_provider'] (not
py_config.get(...)) and use that value to look up load_source_providers() (via
load_source_providers()[py_config['source_provider']] or similar) before
extracting the 'type' field, so a missing py_config['source_provider'] raises
immediately; reference the existing symbols _SOURCE_PROVIDER_TYPE,
load_source_providers and py_config when making this change.

In `@tests/test_mtv_warm_migration.py`:
- Line 18: The code silently defaults when source_provider is missing: replace
the safe-get pattern in the _SOURCE_PROVIDER_TYPE assignment by directly
indexing py_config['source_provider'] and accessing load_source_providers() to
avoid using .get(..., ""); if the key is absent raise a clear ConfigurationError
(or ValueError) with a descriptive message so tests fail fast; update references
around _SOURCE_PROVIDER_TYPE, load_source_providers, and py_config to use
explicit access and add the explicit error handling when the provider is not
found.

In `@utilities/utils.py`:
- Around line 42-56: Update the load_source_providers function docstring to
include an Args section (documenting providers_file: Path or that it reads from
".providers.json"), a Returns section (already present but ensure format matches
project style), and a Raises section (already present but ensure format matches
style), and remove the redundant explicit open mode by changing the file open
call from open(providers_file, "r") to open(providers_file) in the function
body; reference symbols: load_source_providers, providers_file,
MissingProvidersFileError.

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling load_source_providers() silently returns an empty dict when .providers.json is missing and does not handle invalid JSON. This can hide configuration problems and lead to confusing downstream errors (e.g., provider type resolving to None and markers/fixtures behaving unexpectedly). Consider raising a dedicated exception for missing/empty/invalid files (or at least logging and raising a clear error) to fail fast and make CI/debugging deterministic. def load_source_providers() -> dict[str, dict[str, Any]]: """Load source providers from .providers.json. Returns: dict[str, dict[str, Any]]: Provider configurations keyed by provider name. """ providers_file = Path(".providers.json") if not providers_file.exists(): return {} with open(providers_file) as fd: return json.load(fd) Import Bug The module imports py_config from pytest_testconfig, which is inconsistent with other files that use from pytest_testconfig import config as py_config. If pytest_testconfig does not export py_config, this will break test collection. Align the import style across tests to ensure configuration is read correctly. import pytest from ocp_resources.network_map import NetworkMap from ocp_resources.plan import Plan from ocp_resources.provider import Provider from ocp_resources.storage_map import StorageMap from pytest_testconfig import py_config from utilities.migration_utils import get_cutover_value from utilities.mtv_migration import ( create_plan_resource, execute_migration, get_network_migration_map, get_storage_migration_map, ) from utilities.post_migration import check_vms from utilities.utils import get_value_from_py_config, load_source_providers, populate_vm_ids _SOURCE_PROVIDER_TYPE = load_source_providers().get(py_config.get("source_provider", ""), {}).get("type") Collection Side-Effects _SOURCE_PROVIDER_TYPE is computed at import time by reading .providers.json. Import-time file IO is brittle during --collectonly, --setupplan, or environments where the file is not mounted yet, and can cause inconsistent skip behavior (type becomes None). Prefer resolving provider type via a fixture or lazy evaluation inside markers/fixtures to avoid collection-time side effects. from utilities.utils import load_source_providers LOGGER = get_logger(__name__) _SOURCE_PROVIDER_TYPE = load_source_providers().get(py_config.get("source_provider", ""), {}).get("type") # Error message template for simultaneous migration validation EARLY_COMPLETION_MSG = ( "Plan {plan_num} reached {completed_status} before both plans were executing simultaneously. " "Other plan status: {other_status}" )

---

💡 Tool usage guide:

---

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

• When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@utilities/utils.py`:
- Around line 47-52: The function that reads providers_file (providers_file =
Path(".providers.json") ... with open(providers_file) as fd: return
json.load(fd)) can raise json.JSONDecodeError for an empty or invalid file; wrap
the json.load call in a try/except that catches json.JSONDecodeError (and
optionally ValueError) and returns an empty dict {} on decode errors so callers
get {} instead of an exception; reference the providers_file variable and the
json.load call when applying the change.

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling Provider loading uses json parsing without handling invalid JSON; a malformed .providers.json will raise an unhandled JSONDecodeError during execution/import. Consider catching parsing errors and raising a domain-specific exception with actionable context. def load_source_providers() -> dict[str, dict[str, Any]]: """Load source providers from .providers.json. Returns: dict[str, dict[str, Any]]: Provider configurations keyed by provider name. """ providers_file = Path(".providers.json") if not providers_file.exists(): return {} with open(providers_file) as fd: content = fd.read() if not content.strip(): return {} return json.loads(content) Import Side Effects Source providers are loaded at module import time to compute the provider type for skip markers. This performs file I/O during collection and can silently degrade to None/empty values when the file is missing, causing tests to be skipped unexpectedly rather than failing clearly. wait_for_migration_complate, ) from utilities.naming import sanitize_kubernetes_name from utilities.post_migration import check_vms from utilities.resources import create_and_store_resource from utilities.ssh_utils import SSHConnectionManager from utilities.utils import load_source_providers LOGGER = get_logger(__name__) _SOURCE_PROVIDER_TYPE = load_source_providers().get(py_config.get("source_provider", ""), {}).get("type") # Error message template for simultaneous migration validation EARLY_COMPLETION_MSG = ( "Plan {plan_num} reached {completed_status} before both plans were executing simultaneously. " "Other plan status: {other_status}" ) Skip Logic Risk Provider type resolution happens at import time and may become None if the providers file is absent/empty, which can lead to misleading skip reasons and potentially incorrect marker behavior. Prefer resolving in a fixture or ensuring missing configuration fails fast. from utilities.utils import get_value_from_py_config, load_source_providers, populate_vm_ids _SOURCE_PROVIDER_TYPE = load_source_providers().get(py_config.get("source_provider", ""), {}).get("type") pytestmark = [ pytest.mark.skipif( _SOURCE_PROVIDER_TYPE in (Provider.ProviderType.OPENSTACK, Provider.ProviderType.OPENSHIFT, Provider.ProviderType.OVA), reason=f"{_SOURCE_PROVIDER_TYPE} warm migration is not supported.", ), ] # Only apply Jira marker for RHV - skip if issue unresolved, run normally if resolved if _SOURCE_PROVIDER_TYPE == Provider.ProviderType.RHV: pytestmark.append(pytest.mark.jira("MTV-2846", run=False))

---

💡 Tool usage guide:

---

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

• When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

[redhat-qe-bot2]

Failed to build and push ghcr.io/redhatqe/mtv-api-tests:latest