Skip to content

Commit

Permalink
Fix latent bug in manifest repo
Browse files Browse the repository at this point in the history
If a resuable action happened to be the last job to execute, then the
repo in the manifest file was set to the action's repo, not the study's
repo.

However, we no longer need the repo in the manifest files, so just set
it none to avoid this problem.

Once we have a releases UI, we can get rid of the manifest file code
alltogether.
  • Loading branch information
bloodearnest committed Aug 31, 2023
1 parent 430a9a9 commit 6d7d502
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 12 deletions.
3 changes: 2 additions & 1 deletion jobrunner/executors/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,8 @@ def persist_outputs(job_definition, outputs, job_metadata):
write_manifest_file(
medium_privacy_dir,
{
"repo": job_definition.study.git_repo_url,
# this currently needs to exist, but is not used
"repo": None,
"workspace": job_definition.workspace,
},
)
Expand Down
17 changes: 6 additions & 11 deletions tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,8 @@ def test_integration_with_cohortextractor(
# alongside cohortextractor in this test, however once it supports a close
# enough set of dummy data we can merge them into a single test.
extraction_tool = "cohortextractor"

if extraction_tool == "cohortextractor":
generate_action = "generate_cohort"
else:
generate_action = "generate_dataset"
generate_action = "generate_cohort"
image = "cohortextractor"

api = get_executor_api()

Expand All @@ -56,10 +53,6 @@ def test_integration_with_cohortextractor(
# Disable repo URL checking so we can run using a local test repo
monkeypatch.setattr("jobrunner.config.ALLOWED_GITHUB_ORGS", None)

if extraction_tool == "cohortextractor":
image = "cohortextractor"
else:
image = "databuilder:v0.36.0"
ensure_docker_images_present(image, "python")

# Set up a mock job-server with a single job request
Expand Down Expand Up @@ -96,6 +89,7 @@ def test_integration_with_cohortextractor(
# Check that expected number of pending jobs are created
jobs = get_posted_jobs(requests_mock)
assert [job["status"] for job in jobs.values()] == ["pending"] * 7

# Execute one tick of the run loop and then sync
jobrunner.run.handle_jobs(api)
jobrunner.sync.sync()
Expand Down Expand Up @@ -142,6 +136,7 @@ def test_integration_with_cohortextractor(
jobrunner.sync.sync()

# Run the main loop until there are no jobs left and then sync

jobrunner.run.main(exit_callback=lambda active_jobs: len(active_jobs) == 0)
jobrunner.sync.sync()

Expand All @@ -168,7 +163,7 @@ def test_integration_with_cohortextractor(
manifest_file = medium_privacy_workspace / "metadata" / "manifest.json"
manifest = json.loads(manifest_file.read_text())
assert manifest["workspace"] == "testing"
assert manifest["repo"] == str(test_repo.path)
assert manifest["repo"] is None

if extraction_tool == "cohortextractor":
output_name = "input"
Expand Down Expand Up @@ -335,7 +330,7 @@ def test_integration_with_databuilder(
manifest_file = medium_privacy_workspace / "metadata" / "manifest.json"
manifest = json.loads(manifest_file.read_text())
assert manifest["workspace"] == "testing"
assert manifest["repo"] == str(test_repo.path)
assert manifest["repo"] is None

# Check that all the outputs have been produced
assert (high_privacy_workspace / "output/dataset.csv").exists()
Expand Down

0 comments on commit 6d7d502

Please sign in to comment.