search

OpenFn / kit

The bits & pieces that make OpenFn work. (diagrammer, cli, compiler, runtime, runtime manager, logger, etc.)
10 stars 9 forks source link

CLI: adding more validation to workflow #696

Closed SatyamMattoo closed 4 months ago

SatyamMattoo commented 4 months ago

Short Description

Added functions to validate the workflow, steps and options and return errors or warnings accordingly

Related issue

Fixes #681

Implementation Details

Added more validation functions to validate-plan.ts returning errors or warnings for a bad workflow structure.

QA Notes

The following approach is not breaking any existing test cases. But we can add new test cases where we can check the warning logs are printed successfully

Checklist before requesting a review

josephjclark commented 4 months ago

Hi @SatyamMattoo , just to check, this and other issues aren't covered by the DMP programme. This has no relation to c4gt and does not influence any DMP application you make.

If you want to take on a project under DMP - and I hope you will! - you need to apply directly through the C4GT portal. I don't know anything about that, I'm not involved in the process at all.

SatyamMattoo commented 4 months ago

Hey @josephjclark Sorry for the misunderstanding, I forgot to change the branch name here because I was working on a different issue as well. I have applied for the DMP program as well and this isn't related to that. You can have a look and suggest any changes that might be needed here.

Best regards!

josephjclark commented 4 months ago

@SatyamMattoo absolutely fine - the branch name confused me and I just wanted to be clear.

I've got half a review ready but I've been busy. I'll get back to you soon.

SatyamMattoo commented 4 months ago

Hello @josephjclark,

I have made the requested changes. Please review them at your convenience and let me know if there are any additional modifications needed.

Best regards.

josephjclark commented 4 months ago

Thank you @SatyamMattoo - I'll get back to you next week when I have time :)

josephjclark commented 4 months ago

Hi @SatyamMattoo actually I've found a spare 20 minutes so I'm taking a look now.

I don't think you've seem my main comment on the last review. We need to move these new validation steps out of the runtime and into the CLI.

SatyamMattoo commented 4 months ago

Hi @josephjclark I have moved the validation from the runtime to the CLI and also added a few more tests for the logs. You can have a look at your convenience and let me know if there are any more changes needed.

Best regards.

SatyamMattoo commented 4 months ago

Hey @josephjclark, I have committed the requested changes. You can review them at your convenience and let me know if there are any further changes required.

Sorry for the late response, I was occupied with my exams recently.

Best regards.

josephjclark commented 4 months ago

Thank you @SatyamMattoo - this will be released shortly!