Skip to content

fix: return ContractVerificationResult on upload failure in verify_co…#2736

Open
reachsridhard wants to merge 2 commits into
sodadata:mainfrom
reachsridhard:fix/verify-return-type-on-upload-failure
Open

fix: return ContractVerificationResult on upload failure in verify_co…#2736
reachsridhard wants to merge 2 commits into
sodadata:mainfrom
reachsridhard:fix/verify-return-type-on-upload-failure

Conversation

@reachsridhard
Copy link
Copy Markdown

Description

  • Fixes a wrong return type in SodaCloud.verify_contract_on_runner() when the contract upload to Soda Cloud fails.
  • Previously, the method returned an empty list [] on upload failure, while the function is annotated (and expected) to return a ContractVerificationResult. This could cause runtime errors or subtle type bugs for callers handling the result.

Problem being solved

  • On a failed contract upload (missing fileId in response), verify_contract_on_runner() returned [] instead of a ContractVerificationResult.

Expected impact on downstream packages/services

  • Very low risk. This only changes behavior on an error path (upload failure).
  • Return type is now consistent with the annotation and with the surrounding call sites' expectations.
  • No CLI changes, no external API changes, and no plugin interfaces changed.

Changes

  • In soda-core/src/soda_core/common/soda_cloud.py:

    • On missing fileId after uploading the contract YAML, set verification_result.sending_results_to_soda_cloud_failed = True and return verification_result instead of [].
  • Tests:

    • Added test_verify_contract_on_runner_returns_result_when_upload_fails in soda-tests/tests/unit/test_soda_cloud.py.
    • The test simulates:
      • Permission check allowed.
      • Upload returns 200 without fileId (upload failure).
      • Asserts a ContractVerificationResult is returned and sending_results_to_soda_cloud_failed is True.

How I verified

  • Ran the targeted unit test:
    • pytest -q soda-tests/tests/unit/test_soda_cloud.py::test_verify_contract_on_runner_returns_result_when_upload_fails
    • Result: passed.

Affected areas

  • Remote verification flow via Soda Runner only in the upload-failure path.
  • No effect on successful paths or local verification flows.

Backward compatibility and risks

  • Backward compatible for correct callers; previously returning [] was a bug.
  • Risk is limited to making the error path consistent and type-safe.
  • No dependency changes, no schema or protocol changes.

Files changed

  • soda-core/src/soda_core/common/soda_cloud.py
  • soda-tests/tests/unit/test_soda_cloud.py

Checklist

  • I added a test to verify the new functionality.
  • I verified this PR does not break soda-extensions (no interface changes; low risk).

Release notes

  • Fix: SodaCloud.verify_contract_on_runner() now returns a ContractVerificationResult (with sending_results_to_soda_cloud_failed=True) when the contract upload fails, instead of returning [].

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jun 3, 2026

CLA assistant check
All committers have signed the CLA.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 3, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants