Y24-121 - Refactoring validations for pipelines

[dasunpubudumal]

Closes #4135

Changes proposed in this pull request

• Adding a validator_class_name column into pipelines table through a migration.
• Updating batch.rb model to dynamically import validation classes from the corresponding pipelines association.

Instructions for Reviewers

[All PRs] - Confirm PR template filled
[Feature Branches] - Review code
[Production Merges to main]
    - Check story numbers included
    - Check for debug code
    - Check version

[dasunpubudumal]

@BenTopping @yoldas I've seen an intermittent failure of tests in CI. The screenshot below shows CI actions that ran for the same commit (one for the push event, the other for pull_request event).

Any idea why?

[dasunpubudumal]

Couple comments but overall on the right track.

Just to clarify - this implementation has a base BatchValidator for all pipelines and if you want to include custom validation on top of that you add a validator class and include that in the pipeline validator_class_name. Is there a way to override the default BatchValidator validations if we wanted to?

@BenTopping Interesting point. As we've done it currently, there's no overriding the batch validator. This implementation just adds additional validations on top of the existing ones.

Do we need to override it? If so, we can think of something that facilitates that.

[BenTopping]

Couple comments but overall on the right track. Just to clarify - this implementation has a base BatchValidator for all pipelines and if you want to include custom validation on top of that you add a validator class and include that in the pipeline validator_class_name. Is there a way to override the default BatchValidator validations if we wanted to?

@BenTopping Interesting point. As we've done it currently, there's no overriding the batch validator. This implementation just adds additional validations on top of the existing ones.

Do we need to override it? If so, we can think of something that facilitates that.

I am not sure, just thinking about how we would implement the next novaseqX story with this structure.

[dasunpubudumal]

Couple comments but overall on the right track. Just to clarify - this implementation has a base BatchValidator for all pipelines and if you want to include custom validation on top of that you add a validator class and include that in the pipeline validator_class_name. Is there a way to override the default BatchValidator validations if we wanted to?

@BenTopping Interesting point. As we've done it currently, there's no overriding the batch validator. This implementation just adds additional validations on top of the existing ones. Do we need to override it? If so, we can think of something that facilitates that.

I am not sure, just thinking about how we would implement the next novaseqX story with this structure.

@BenTopping So, if we want to override the batch validation (which is BatchCreationValidator in the current implementation) we would have to do it based on the pipeline, isn't it?

One option that I'm thinking of is to add a toggle (a boolean somewhere) for the batch model to determine whether to use BatchCreationValidator, or to use the validator specified in validator_class_name. We could add this toggle (say, use_default_validator) into the pipelines table, with the following outcomes:

1. If use_default_validator is true, the batch would use BatchCreationValidator, and a validator specified in validator_class_name (if specified).
2. If use_default_validator is false, it would not use BatchCreationValidator, and would use the validator listed under validator_class_name. We would have to be careful in this case to have a validator_class_name as this would disable current validations entirely.

What do you think about this approach? We do not necessarily have to add use_default_validator in the table per-se; we could add it in a config or somewhere (which I'm not entirely sure whether if it's appropriate).

[qlty-cloud-legacy[bot]]

Code Climate has analyzed commit d84c7cb4 and detected 7 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Complexity	7

The test coverage on the diff in this pull request is 87.8% (50% is the threshold).

This pull request will bring the total coverage in the repository to 86.7% (0.0% change).

View more on Code Climate.