test(ingestion): cover newest-version selection in the Mlflow source

[IceS2]

Follow-up to #30191, addressing a Greptile review comment: the Mlflow integration test created a single model version, so max(..., key=version) was never exercised — a matcher that selected a stale version would still pass.

What

Register a second model version with distinct hyperparameters (alpha=0.2, l1_ratio=0.3), then assert the ingested entity carries the newest version's run (alpha=0.5, l1_ratio=1.0) and that the older version's values are absent. The log-model retry loop is extracted into a shared helper so both versions register through the same path.

Key finding

RegisteredModel.latest_versions returns a single element — the newest version — even with many versions registered (verified against MLflow 3.10 and 3.14: three versions still yield a one-element list). Stages were the only thing that produced multiple entries, and they are deprecated. So the connector's max() cannot be discriminated by min-vs-max no matter how many versions exist. What the second version does change is which run the connector reads (get_run(latest_version.run_id)), so asserting the ingested hyperparameters match the newest run is the meaningful coverage — it catches a regression that walked all versions and grabbed the oldest.

Verified locally against the testcontainers stack: the strengthened test fails with the pre-fix connector (Version not found) and passes with #30191's fix.

Note

This branch is stacked on #30191 and currently contains its commits too; the diff narrows to just the test once #30191 merges. Draft until then.

[gitar-bot]

💡 Quality: Duplicated update_registered_model call and comment

Lines 179-181 are a verbatim copy of the block at 175-177 (same comment, same update_registered_model call). The second call is redundant — it re-applies the identical description and adds no coverage. Remove the duplicated block to keep the fixture clean.

Remove the duplicated update_registered_model block (lines 179-181).:

# Describing the model bumps RegisteredModel.last_updated_timestamp past the
# version's, which must not hide the version from the registry listing.
mlflow.MlflowClient().update_registered_model(MODEL_NAME, description=MODEL_DESCRIPTION)

• Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

[gitar-bot]

Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Strengthens MLflow ingestion tests by asserting the latest model version's run parameters. Consolidate the duplicated update_registered_model call and accompanying comment on lines 175-181.

💡 Quality: Duplicated update_registered_model call and comment

📄 ingestion/tests/integration/sources/mlmodels/mlflow/test_mlflow.py:175-181

Lines 179-181 are a verbatim copy of the block at 175-177 (same comment, same update_registered_model call). The second call is redundant — it re-applies the identical description and adds no coverage. Remove the duplicated block to keep the fixture clean.

Remove the duplicated update_registered_model block (lines 179-181).

# Describing the model bumps RegisteredModel.last_updated_timestamp past the
# version's, which must not hide the version from the registry listing.
mlflow.MlflowClient().update_registered_model(MODEL_NAME, description=MODEL_DESCRIPTION)

🤖 Prompt for agents

Code Review: Strengthens MLflow ingestion tests by asserting the latest model version's run parameters. Consolidate the duplicated `update_registered_model` call and accompanying comment on lines 175-181.

1. 💡 Quality: Duplicated update_registered_model call and comment
   Files: ingestion/tests/integration/sources/mlmodels/mlflow/test_mlflow.py:175-181

   Lines 179-181 are a verbatim copy of the block at 175-177 (same comment, same `update_registered_model` call). The second call is redundant — it re-applies the identical description and adds no coverage. Remove the duplicated block to keep the fixture clean.

   Fix (Remove the duplicated update_registered_model block (lines 179-181).):
   # Describing the model bumps RegisteredModel.last_updated_timestamp past the
   # version's, which must not hide the version from the registry listing.
   mlflow.MlflowClient().update_registered_model(MODEL_NAME, description=MODEL_DESCRIPTION)

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}