Skip to content

Commit f01c12a

Browse files
authored
Merge pull request #2270 from broadinstitute/jb-anndata-ingest-bugs
Prevent errors when assembling ingestSummary properties (SCP-6015)
2 parents 1d9400b + 5947f00 commit f01c12a

File tree

2 files changed

+75
-3
lines changed

2 files changed

+75
-3
lines changed

app/models/ingest_job.rb

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1064,9 +1064,29 @@ def anndata_summary_props
10641064
pipeline_args.detect { |c| c == study_file.id.to_s } &&
10651065
(pipeline_args & CORE_ACTIONS).any?
10661066
end
1067+
if previous_jobs.empty?
1068+
run = get_ingest_run
1069+
perftime = (TimeDifference.between(
1070+
event_timestamp(run.create_time), event_timestamp(run.update_time)
1071+
).in_seconds * 1000).to_i
1072+
status = run.status.state == "FAILED" ? 'failed' : 'success'
1073+
return {
1074+
perfTime: perftime,
1075+
fileName: study_file.name,
1076+
fileType: study_file.file_type,
1077+
fileSize: study_file.upload_file_size,
1078+
studyAccession: study.accession,
1079+
trigger: study_file.upload_trigger,
1080+
jobStatus: status,
1081+
numFilesExtracted: 0,
1082+
machineType: params_object.machine_type,
1083+
action: 'unknown',
1084+
exitCode: exit_code
1085+
}
1086+
end
10671087
# get total runtime from initial extract to final parse
1068-
initial_extract = previous_jobs.last
1069-
final_parse = previous_jobs.first
1088+
initial_extract = previous_jobs.min_by(&:create_time)
1089+
final_parse = previous_jobs.max_by(&:create_time)
10701090
start_time = event_timestamp(initial_extract.create_time)
10711091
end_time = event_timestamp(final_parse.update_time) # update_time is a proxy for last event timestamp
10721092
job_perftime = (TimeDifference.between(start_time, end_time).in_seconds * 1000).to_i
@@ -1081,7 +1101,7 @@ def anndata_summary_props
10811101
if job_status == 'failed'
10821102
first_failure = previous_jobs.reverse.detect { |job| client.job_error(job.name).present? }
10831103
args = client.get_job_command_line(job: first_failure)
1084-
error_action = args.detect {|c| BatchApiClient::FILE_TYPES_BY_ACTION.keys.include?(c.to_sym) }
1104+
error_action = args.detect { |c| BatchApiClient::FILE_TYPES_BY_ACTION.keys.include?(c.to_sym) }
10851105
code = client.exit_code_from_task(first_failure.name)
10861106
end
10871107
# count total number of files extracted

test/models/ingest_job_test.rb

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -628,6 +628,58 @@ class IngestJobTest < ActiveSupport::TestCase
628628
end
629629
end
630630

631+
test 'should handle not finding upstream jobs in ingestSummary' do
632+
study = FactoryBot.create(:detached_study,
633+
name_prefix: 'Empty Jobs test',
634+
user: @user,
635+
test_array: @@studies_to_clean)
636+
ann_data_file = FactoryBot.create(:ann_data_file, name: 'failed.h5ad', study:)
637+
pipeline_name = SecureRandom.uuid
638+
dummy_job = Google::Apis::BatchV1::Job.new(
639+
name: pipeline_name,
640+
create_time: (@now - 1.hour).to_s,
641+
update_time: @now.to_s,
642+
status: Google::Apis::BatchV1::JobStatus.new(
643+
state: 'FAILED',
644+
status_events: [
645+
Google::Apis::BatchV1::StatusEvent.new(event_time: (@now - 1.hour).to_s),
646+
Google::Apis::BatchV1::StatusEvent.new(event_time: @now.to_s)
647+
]
648+
)
649+
)
650+
list_mock = Minitest::Mock.new
651+
list_mock.expect :jobs, []
652+
mock = Minitest::Mock.new
653+
mock.expect :list_jobs, list_mock
654+
mock.expect :exit_code_from_task, 1, [pipeline_name]
655+
params_object = AnnDataIngestParameters.new(
656+
anndata_file: "gs://#{study.bucket_id}/matrix.h5ad", file_size: ann_data_file.upload_file_size
657+
)
658+
job = IngestJob.new(
659+
pipeline_name:, study:, study_file: ann_data_file, user: @user, action: :ingest_anndata, params_object:
660+
)
661+
expected_props = {
662+
perfTime: 3_600_000,
663+
fileName: ann_data_file.name,
664+
fileType: ann_data_file.file_type,
665+
fileSize: ann_data_file.upload_file_size,
666+
studyAccession: study.accession,
667+
trigger: ann_data_file.upload_trigger,
668+
jobStatus: 'failed',
669+
numFilesExtracted: 0,
670+
machineType: params_object.machine_type,
671+
action: 'unknown',
672+
exitCode: 1
673+
}.with_indifferent_access
674+
job.stub :get_ingest_run, dummy_job do
675+
ApplicationController.stub :batch_api_client, mock do
676+
props = job.anndata_summary_props
677+
assert_equal expected_props, props.with_indifferent_access
678+
mock.verify
679+
end
680+
end
681+
end
682+
631683
test 'should limit size when reading error logfile for email' do
632684
job = IngestJob.new(study: @basic_study, study_file: @basic_study_exp_file, user: @user, action: :ingest_expression)
633685
file_location = @basic_study_exp_file.bucket_location

0 commit comments

Comments
 (0)