Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Graphql: Submit Workflow Execution #838

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

JeffreyThiessen
Copy link
Member

@JeffreyThiessen JeffreyThiessen commented Nov 7, 2024

What does this PR do and why?

Describe in detail what your merge request does and why.

  • Added new graphql mutation SubmitWorkflowExecution which submits a workflow execution / pipeline.
  • Added a new Validator for WorkflowExecution objects, which ensures samplesheet params attachments are formatted correctly.
    • This is not an issue when workflow executions are submitted via the Web UI, but graphql accepts any text so an invalid attachment could cause problems in the pipeline if not validated beforehand.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other pull requests.

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

Get some data to use in a submission

query getProject{
  project(fullPath: "bacillus/bacillus-anthracis/outbreak-2022") {
    id
    name
    path
    samples(first:1){
      nodes{
        name
        id
        puid
        attachments{
          nodes{
            filename
            id
          }
        }
      }
    }
  }
}

Use the data returned from the previous step to populate the following submission mutation

mutation submitWE {
  submitWorkflowExecution (input:{
    name:"My Workflow from graphql 5"
    projectId: "gid://irida/Project/13d4c370-c707-4afa-8fe2-a26719508c02"
    updateSamples: false
    emailNotification: false
    workflowName: "phac-nml/iridanextexample"
    workflowVersion:"1.0.3"
    workflowParams: {
      assembler: "stub",
      random_seed: 1,
      project_name: "assembly"
    }
    workflowType: "NFL"
    workflowTypeVersion:"DSL2"
    workflowEngine:"nextflow"
    workflowEngineVersion: "23.10.0"
    workflowEngineParameters: [
      {
        key:"-r",
        value:"1.0.3"
      }
    ]
    workflowUrl: "https://github.com/phac-nml/iridanextexample"
    samplesWorkflowExecutionsAttributes:[
      {
        sample_id:"gid://irida/Sample/e978e898-f5f6-44a1-934a-418392d4c532"
        samplesheet_params:{
          sample: "INXT_SAM_AYMK34F2FG",
          fastq_1:"gid://irida/Attachment/dd4470cb-71b2-4d75-accf-f6954cc4c922",
          fastq_2:"gid://irida/Attachment/8299fc62-66fc-4915-959e-f4f97d51b061"
        }
      }
    ]
  }){
    workflowExecution{
      name
      id
    }
    errors{
      message
      path
    }
  }
}

PR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

@JeffreyThiessen JeffreyThiessen force-pushed the graphql/submit_workflow_execution branch from eb52de5 to 815d739 Compare December 10, 2024 20:34
@JeffreyThiessen JeffreyThiessen changed the base branch from main to graphql/workflow_execution_type December 10, 2024 20:34
@JeffreyThiessen JeffreyThiessen force-pushed the graphql/workflow_execution_type branch from 0230b2e to 6f79351 Compare December 18, 2024 16:39
@JeffreyThiessen JeffreyThiessen force-pushed the graphql/submit_workflow_execution branch 2 times, most recently from ce227e4 to abd7c59 Compare December 19, 2024 16:31
Base automatically changed from graphql/workflow_execution_type to main December 19, 2024 17:03
@JeffreyThiessen JeffreyThiessen force-pushed the graphql/submit_workflow_execution branch from abd7c59 to f463a13 Compare December 19, 2024 17:17
@JeffreyThiessen JeffreyThiessen marked this pull request as ready for review December 19, 2024 17:27
@JeffreyThiessen JeffreyThiessen force-pushed the graphql/submit_workflow_execution branch from 3232e31 to de21328 Compare December 30, 2024 19:56
@JeffreyThiessen JeffreyThiessen force-pushed the graphql/submit_workflow_execution branch from de21328 to ca2516f Compare January 7, 2025 19:58

This comment has been minimized.

@JeffreyThiessen JeffreyThiessen self-assigned this Jan 8, 2025
@JeffreyThiessen JeffreyThiessen added ready for review Pull request is ready for review graphql GraphQL API related changes labels Jan 8, 2025

This comment has been minimized.

ChrisHuynh333
ChrisHuynh333 previously approved these changes Jan 9, 2025
Copy link
Collaborator

@ChrisHuynh333 ChrisHuynh333 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and works as described!

Copy link
Member

@ericenns ericenns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried resetting the database on this branch but it failed due to the new validation of samplesheet_params can you please update the seed_workflow_executions method in db/seeds.rb so that this error no longer occurs.

Note: I only figured out that was the case after changing WorkflowExecution.create to WorkflowExecution.create! which causes an exception to be raised when the record is unable to be saved. Without that it was failing on line 197 in db/seeds.rb as it could not create the attachment without attachable_id

@JeffreyThiessen
Copy link
Member Author

I tried resetting the database on this branch but it failed due to the new validation of samplesheet_params can you please update the seed_workflow_executions method in db/seeds.rb so that this error no longer occurs.

Note: I only figured out that was the case after changing WorkflowExecution.create to WorkflowExecution.create! which causes an exception to be raised when the record is unable to be saved. Without that it was failing on line 197 in db/seeds.rb as it could not create the attachment without attachable_id

Fixed in 02e498e

@JeffreyThiessen JeffreyThiessen force-pushed the graphql/submit_workflow_execution branch from 02e498e to ca3d284 Compare January 10, 2025 18:36

This comment has been minimized.

Copy link
Member

@ericenns ericenns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good. Just a few comments and could you review the rubocop warnings present in the Files changed tab.

app/graphql/mutations/submit_workflow_execution.rb Outdated Show resolved Hide resolved
app/graphql/mutations/submit_workflow_execution.rb Outdated Show resolved Hide resolved
app/graphql/mutations/submit_workflow_execution.rb Outdated Show resolved Hide resolved
@JeffreyThiessen JeffreyThiessen force-pushed the graphql/submit_workflow_execution branch from 08fa191 to 7c12f74 Compare January 13, 2025 16:01

This comment has been minimized.

@JeffreyThiessen JeffreyThiessen force-pushed the graphql/submit_workflow_execution branch from 7c12f74 to e7fff0f Compare January 13, 2025 22:43
Copy link

Code Metrics Report

Coverage Test Execution Time
92.9% 10m6s

Code coverage of files in pull request scope (96.9%)

Files Coverage
app/controllers/workflow_executions_controller.rb 96.6%
app/graphql/mutations/submit_workflow_execution.rb 92.7%
app/graphql/types/mutation_type.rb 100.0%
app/models/workflow_execution.rb 100.0%
app/validators/workflow_execution_samplesheet_params_validator.rb 100.0%

Reported by octocov

@deepsidhu85
Copy link
Contributor

deepsidhu85 commented Jan 15, 2025

While testing this PR out, I was able to use this for the samplesWorkflowExecutionAttributes JSON for the mutation which worked but it shouldn't have

{"samples_workflow_executions_attributes": {"sample_id": "gid://irida/Sample/c9427c0f-51e2-45b8-828c-6d32113eb679","samplesheet_params": {"sample": "INXT_SAM_AZBO256QCZ","fastq_1": "gid://irida/Attachment/141ea970-db29-4a97-9953-937986ff3ec5","fastq_2": "gid://irida/Attachment/f1179ccd-2308-4fb5-bb8b-25a900e387b5"}}}

Looks like we need to add validation to make sure that the sample_id and sample belong to the project that was set for the mutation, and validate that the attachments belong to this sample.

The reason the above shouldn't have worked is because the sample_id and sample don't belong to the project that was set in the mutation, and the attachments also do not belong to this sample

Steps to recreate:

Seed db if you already don't have it seeded
Login as [email protected]
Create a new project, sample, and upload a paired-end dataset
Run the submitWorkflowExecution mutation using http://localhost:3000/graphiql with the changes listed below the mutation:

 mutation($samples_workflow_executions_attributes: [JSON!]!){
      submitWorkflowExecution( input:{
        name: "testWE"
        projectId: "gid://irida/Project/b92623f7-6dc5-4ab6-a520-bf539b46a78a"
        updateSamples: false
        emailNotification: false
        workflowName: "phac-nml/iridanextexample"
        workflowVersion: "1.0.3"
        workflowParams: {
          assembler: "stub",
          random_seed: 1,
          project_name: "assembly"
        }
        workflowType: "NFL"
        workflowTypeVersion:"DSL2"
        workflowEngine:"nextflow"
        workflowEngineVersion: "23.10.0"
        workflowEngineParameters: [
          {
            key:"-r",
            value:"1.0.3"
          }
        ]
        workflowUrl: "https://github.com/phac-nml/iridanextexample"
        samplesWorkflowExecutionsAttributes: $samples_workflow_executions_attributes
        }) {
        workflowExecution{
          name
          id
        }
        errors{
          message
          path
        }
      }
    }

set the project_id to global id of created project above

In the hash for samplesWorkflowExecutionsAttributes, set the sample_id to a global id of a sample from another project, and set the sample with the puid of this sample. Set the fastq_1 and fastq_2 to the global ids of the attachments you uploaded above

Run the mutation and it should succeed in launching the workflow execution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
graphql GraphQL API related changes ready for review Pull request is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants