Merge pull request #667 from opensafely-core/level-4-check-fixes

level 4 check fixes

jobrunner/config.py

@@ -157,6 +157,7 @@ def database_urls_from_env(env):
     ".html",
     ".pdf",
     ".txt",
+    ".log",
     ".json",
 ]
 

jobrunner/executors/local.py

@@ -415,8 +415,10 @@ def finalize_job(job_definition):
     if job_definition.cancelled:
         write_job_logs(job_definition, job_metadata, copy_log_to_workspace=False)
     else:
-        write_job_logs(job_definition, job_metadata, copy_log_to_workspace=True)
         excluded = persist_outputs(job_definition, results.outputs, job_metadata)
+        write_job_logs(
+            job_definition, job_metadata, copy_log_to_workspace=True, excluded=excluded
+        )
         results.level4_excluded_files.update(**excluded)
 
     RESULTS[job_definition.id] = results
@@ -443,11 +445,13 @@ def get_job_metadata(job_definition, outputs, container_metadata):
     return job_metadata
 
 
-def write_job_logs(job_definition, job_metadata, copy_log_to_workspace=True):
+def write_job_logs(
+    job_definition, job_metadata, copy_log_to_workspace=True, excluded=None
+):
     """Copy logs to log dir and workspace."""
     # Dump useful info in log directory
     log_dir = get_log_dir(job_definition)
-    write_log_file(job_definition, job_metadata, log_dir / "logs.txt")
+    write_log_file(job_definition, job_metadata, log_dir / "logs.txt", excluded)
     with open(log_dir / "metadata.json", "w") as f:
         json.dump(job_metadata, f, indent=2)
 
@@ -643,7 +647,7 @@ def get_unmatched_outputs(job_definition, outputs):
     return [filename for filename in all_outputs if filename not in outputs]
 
 
-def write_log_file(job_definition, job_metadata, filename):
+def write_log_file(job_definition, job_metadata, filename, excluded):
     """
     This dumps the (timestamped) Docker logs for a job to disk, followed by
     some useful metadata about the job and its outputs
@@ -659,6 +663,10 @@ def write_log_file(job_definition, job_metadata, filename):
             f.write(f"{key}: {job_metadata[key]}\n")
         f.write("\noutputs:\n")
         f.write(tabulate(outputs, separator="  - ", indent=2, empty="(no outputs)"))
+        if excluded:
+            f.write("\nexcluded files:\n")
+            for excluded_file, msg in excluded.items():
+                f.write(f"{excluded_file}: {msg}")
 
 
 # Keys of fields to log in manifest.json and log file
@@ -776,7 +784,7 @@ def redact_environment_variables(container_metadata):
 
 def write_manifest_file(workspace_dir, manifest):
     manifest_file = workspace_dir / METADATA_DIR / MANIFEST_FILE
-    manifest_file.parent.mkdir(exist_ok=True)
+    manifest_file.parent.mkdir(exist_ok=True, parents=True)
     manifest_file_tmp = manifest_file.with_suffix(".tmp")
     manifest_file_tmp.write_text(json.dumps(manifest, indent=2))
     manifest_file_tmp.replace(manifest_file)

jobrunner/run.py

@@ -466,10 +466,7 @@ def save_results(job, job_definition, results):
         message = "Completed successfully"
 
         if results.level4_excluded_files:
-            files = "\n".join(
-                f"{f}: {msg}" for f, msg in results.level4_excluded_files.items()
-            )
-            message += f", but {len(results.level4_excluded_files)} file(s) marked as moderately_sensitive were excluded:\n{files}"
+            message += f", but {len(results.level4_excluded_files)} file(s) marked as moderately_sensitive were excluded. See job log for details."
 
     set_code(job, code, message, error=error, results=results)
 

tests/test_local_executor.py

@@ -513,6 +513,10 @@ def test_finalize_large_level4_outputs(
     assert "output/output.txt" in txt
     assert "1.0Mb" in txt
     assert "0.5Mb" in txt
+    log_file = level4_dir / "metadata/action.log"
+    log = log_file.read_text()
+    assert "excluded files:" in log
+    assert "output/output.txt: File size of 1.0Mb is larger that limit of 0.5Mb." in log
 
 
 @pytest.mark.needs_docker
@@ -547,6 +551,11 @@ def test_finalize_invalid_file_type(docker_cleanup, job_definition, tmp_work_dir
     txt = message_file.read_text()
     assert "output/output.rds" in txt
 
+    log_file = level4_dir / "metadata/action.log"
+    log = log_file.read_text()
+    assert "excluded files:" in log
+    assert "output/output.rds: File type of .rds is not valid level 4 file" in log
+
 
 @pytest.mark.needs_docker
 def test_finalize_patient_id_header(docker_cleanup, job_definition, tmp_work_dir):
@@ -585,6 +594,11 @@ def test_finalize_patient_id_header(docker_cleanup, job_definition, tmp_work_dir
     assert "output/output.csv" in txt
     assert "patient_id" in txt
 
+    log_file = level4_dir / "metadata/action.log"
+    log = log_file.read_text()
+    assert "excluded files:" in log
+    assert "output/output.csv: File has patient_id column" in log
+
 
 @pytest.mark.needs_docker
 def test_finalize_large_level4_outputs_cleanup(

tests/test_run.py

@@ -597,7 +597,7 @@ def test_handle_job_finalized_success_with_large_file(db):
     assert job.state == State.SUCCEEDED
     assert "Completed successfully" in job.status_message
     assert "were excluded" in job.status_message
-    assert "output/output.csv: too big" in job.status_message
+    assert "output/output.csv: too big" not in job.status_message
 
 
 @pytest.mark.parametrize(