QTZPOC-20

QTZPOC-20: Implement explicit joblib model flavor and enhance model import workflow with extensive E2E tests.

This commit introduces a first-class joblib model flavor for direct loading and saving of Python pickle artifacts, including custom code within the artifact's code/ directory, bypassing MLflow's flavor loaders. The model import workflow's status database configuration is now streamlined to default to the POSTGRES_* server, with explicit IMPORT_STATUS_DB_* overrides.

Comprehensive end-to-end tests have been added for the model import workflow, covering success paths, gate rejections, digest enforcement, provisioning failures, status record integrity, cleanup, and retry/recovery scenarios using a real-clock environment. New E2

e2e/test_import_model_provisioning_failures.py

@@ -13,15 +13,19 @@ failure is always the second kind.
 | `experiment_creation` | the MLflow server is genuinely stopped | broken dependency |
 | `artifact_upload` | targeted patch on `log_artifacts` | patch, justified in place |
 | `registration` | targeted patch on `create_model_version` | patch, justified in place |
-| `model_document` | a document already holding the id the activity will compute, against the real unique index | broken dependency |
+| `model_document` | a real unique index refuses the activity's own insert | broken dependency |
 
-Two patches rather than the one design E7 anticipated. The reason is the same for both and is
-recorded in each test: with one server serving both tracking and registry, and MLflow validating
-nothing about a registered model name except that it is non-empty (which gate 6 already requires),
-there is no way to make the world hostile to *just* one of these two steps. Stopping the server
-fails experiment creation first; a malformed name is not malformed to MLflow. The patch is on the
-client call the step makes, not on the activity or the workflow, so what is being tested is still
-"the workflow attributes this step's failure to this step".
+Two departures from what the tasks anticipated, both recorded in the test that found them:
+
+- **Registration needs a patch too**, not just artifact upload. One process serves both tracking and
+  the registry, so stopping it fails experiment creation three steps earlier; and MLflow validates
+  nothing about a registered model name except that it is non-empty, which gate 6 already requires.
+  Both patches are on the single client call the step *is*, never on the activity or the workflow, so
+  what is under test — the workflow attributing this step's failure to this step — is untouched.
+- **The model document's unique-index violation is on `model_config.target`**, not on `id`. The
+  duplicate-`id` state the task describes cannot be constructed at all; `TestModelDocument`'s
+  docstring shows why, and the mechanism the task wanted — a real index refusing a real insert — is
+  kept intact.
 """
 
 from __future__ import annotations
@@ -292,9 +296,26 @@ class TestRegistration:
 
 
 class TestModelDocument:
-    """The last step, failed by a real unique index on a real collection."""
+    """The last step, failed by a real unique index refusing the real insert.
 
-    async def test_a_duplicate_id_fails_at_model_document(
+    **Why the index is on `model_config.target` and not on `id`.** Task 7.4 asks for "a document
+    already holding the id the activity is about to compute". That state cannot be constructed, and
+    why is worth more than the test would have been: `_next_model_document_id` computes
+    `max(int(id)) + 1` over `distinct('id')` of the *same* collection, so every value that
+    stringifies to the colliding id is counted by that maximum. Put a document at `'8'` and the next
+    id becomes `'9'`. Make the id unparseable and it is ignored by the numbering *and* does not
+    collide. There is no third case — the numbering cannot collide with itself, and the only real
+    duplicate-key path on `id` is two imports racing, which this change lists as a non-goal. That is
+    a good property of the implementation, not a gap.
+
+    So the mechanism the task asked for is kept — a real unique index refusing the activity's own
+    insert — on a key that can actually collide. `model_config.target` is `'SE'` on the seeded
+    documents and `'SE'` on the one this import writes, so a unique index there makes the insert a
+    genuine `DuplicateKeyError` from the server. No patch, no mock: the same class of failure, from
+    the same enforcement mechanism, at the same step.
+    """
+
+    async def test_a_unique_index_violation_fails_at_model_document(
         self,
         import_status_table: Any,
         import_models_collection: Any,
@@ -308,50 +329,47 @@ class TestModelDocument:
         import_envelope: Any,
         import_read_status_row: Any,
     ) -> None:
-        """A document already holding the id `max(int(id)) + 1` will compute, under another name.
-
-        Under another name on purpose: a document with the *same* name would make the activity skip
-        the insert entirely, which is a different behaviour (task 8.6) and would not fail. This one
-        collides only on `id`, so the insert is attempted and the index refuses it.
-
-        No patch and no mock: the failure is `pymongo`'s duplicate-key error from a real index,
-        which is the thing that would happen in production if two imports raced.
-        """
-        collision_id = str(max(int(seeded) for seeded in SEEDED_MODEL_IDS) + 1)
-        import_models_collection.insert_one(
-            {'id': collision_id, 'name': 'a_different_model', 'active': True, 'model_config': {}}
-        )
-
-        handle, import_run_id, object_key = await _upload_and_start(
-            import_bundle_factory=import_bundle_factory,
-            import_upload_bundle=import_upload_bundle,
-            import_insert_status_row=import_insert_status_row,
-            import_build_input=import_build_input,
-            import_execute=import_execute,
-            import_envelope=import_envelope,
-        )
-        with pytest.raises(WorkflowFailureError):
-            await handle.result()
+        # All three seeded documents carry target 'SE', so the index has to be buildable first:
+        # give two of them their own target and leave exactly one holding 'SE'. That one is what the
+        # import's own document — also target 'SE' — collides with.
+        for ordinal, seeded_id in enumerate(sorted(SEEDED_MODEL_IDS)[1:], start=1):
+            import_models_collection.update_one(
+                {'id': seeded_id}, {'$set': {'model_config.target': f'OTHER_{ordinal}'}}
+            )
+        import_models_collection.create_index('model_config.target', unique=True, name='e2e_target')
+        try:
+            handle, import_run_id, object_key = await _upload_and_start(
+                import_bundle_factory=import_bundle_factory,
+                import_upload_bundle=import_upload_bundle,
+                import_insert_status_row=import_insert_status_row,
+                import_build_input=import_build_input,
+                import_execute=import_execute,
+                import_envelope=import_envelope,
+                parameters={'target_variable': 'SE'},
+            )
+            with pytest.raises(WorkflowFailureError):
+                await handle.result()
+            row = import_read_status_row(import_run_id)
+        finally:
+            # The collection is dropped and reseeded per test, but dropping the index here keeps the
+            # failure local even if that ever changes.
+            import_models_collection.drop_index('e2e_target')
 
         _assert_platform_failure(
-            row=import_read_status_row(import_run_id),
+            row=row,
             step=ImportStep.MODEL_DOCUMENT,
             workflow_id=handle.id,
             object_key=object_key,
         )
 
-        # No document for this model, and the colliding one is untouched.
+        # No document for this model, and the seeded ones are exactly as they were.
         assert import_models_collection.find_one({'name': import_names['model_name']}) is None
-        blocker = import_models_collection.find_one({'id': collision_id}, {'_id': False})
-        assert blocker == {
-            'id': collision_id,
-            'name': 'a_different_model',
-            'active': True,
-            'model_config': {},
-        }
+        assert {
+            str(document['id']) for document in import_models_collection.find({}, {'_id': False})
+        } == set(SEEDED_MODEL_IDS)
 
-        # Everything before it stands: "the model was registered, but the settings ... could not be
-        # saved" is the catalog sentence, and this is what makes it true rather than merely stored.
+        # Everything before it stands — registered, and unusable, which is the point of the split
+        # between this step's sentence and the registration step's.
         mlflow_client = _fresh_client(import_mlflow_server)
         versions = list(mlflow_client.search_model_versions(f"name='{import_names['model_name']}'"))
         assert len(versions) == 1