feat: rds error should fail loudly in sfn

[danieljhegeman]

Reason for Change

• feat(SFN Errors): Mark ingest sfn with errors as failed #6234

Changes

• exit(1) when cxg failure or seurat failure is present. Logging is already occurring locally as well as notification to slack.

Testing steps

• Tests will come in follow-up but this should be incorporated asap for schema migration testing purposes.

Checklist 🛎️

• Add product, design, and eng as reviewers for rdev review
• For UI changes, add screenshots/videos, so the reviewers know what you expect them to see
• For UI changes, add e2e tests to prevent regressions

Notes for Reviewer

[github-actions]

Deployment Summary

• backend_url: https://pr-6255-backend.rdev.single-cell.czi.technology
• frontend_url: https://pr-6255-frontend.rdev.single-cell.czi.technology
• delete_db_task_definition_arn: arn:aws:ecs:us-west-2:***:task-definition/dp-rdev-pr-6255-deletion:1
• migrate_db_task_definition_arn: arn:aws:ecs:us-west-2:***:task-definition/dp-rdev-pr-6255-migration:1

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (77e0eb7) 91.76% compared to head (143c604) 91.76%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6255   +/-   ##
=======================================
  Coverage   91.76%   91.76%           
=======================================
  Files         175      175           
  Lines       14129    14129           
=======================================
  Hits        12965    12965           
  Misses       1164     1164           

Flag	Coverage Δ
unittests	91.76% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[nayib-jose-gloria]

This change works for cxg + seurat, but the original ticket mentions marking the sfn as failed upon errors in the validate step also. I'm not 100% sure if we should, since some validate failures will be intended failures (i.e. the curator submits an invalid dataset), but it would be helpful for debugging cases where the failure is unexpected. I'm not sure how simple it'd be to determine whether a validation error is expected or unexpected and handle accordingly. @Bento007 your thoughts?

We can decouple that change to unblock this, if we make a separate ticket for it

[nayib-jose-gloria]

@Bento007, I believe, mentioned that it might be problematic to fail before we deregister the Job Definition. Am I recalling correctly? This step runs before then, so that'd be a problem if so

[danieljhegeman]

@Bento007, I believe, mentioned that it might be problematic to fail before we deregister the Job Definition. Am I recalling correctly? This step runs before then, so that'd be a problem if so

@nayib-jose-gloria that's right, I'll fix that.

[Bento007]

When I was imaging the solution for this, i was assuming we could do it within in the SFN definition. When a step fails in the SFN it puts the error in the result passed to the next step. All we'd need to do is check for the existence of this error field in the resulting JSON and set the state to Failed. This check could then be done right after we deregister. Like this

    "DeregisterJobDefinition": {
      "Type": "Task",
      "Parameters": {
        "JobDefinition.$": "$.batch.JobDefinitionName"
      },
      "Resource": "arn:aws:states:::aws-sdk:batch:deregisterJobDefinition",
      "Next": "Detect Errors"
    },
    "Detect Errors": {
      "Type": "Choice",
      "Choices": [
        {
          "Variable": "$.error",
          "IsPresent": true,
          "Next": "Fail"
        }
      ],
      "Default": "Pass"
    },
    "Fail": {
      "Type": "Fail"
    },
    "Pass": {
      "Type": "Pass",
      "End": true
    }

[Bento007]

Will you also be catching error from download and validate in future work?

[Bento007]

🔥