search

OpenFn / kit

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

Fixing:#192 #670

Closed SatyamMattoo closed 5 months ago

SatyamMattoo commented 5 months ago

Short Description

The fix includes a function responsible for adaptor validation and installation utilizing the existing install function from the repo/handler module.

Related issue

Fixes #192

Implementation Details

To prevent potential failures, I propose implementing an automatic adaptor installation feature. This would validate the presence of the required adaptor and install it if necessary, ensuring a seamless user experience. I've developed and tested the solution thoroughly, including unit tests and compatibility checks with existing integration tests.

Checklist before requesting a review

josephjclark commented 5 months ago

I've noticed out CI tests haven't run on this, I think because it's targeted at main. I wonder if I can address this...

EDIT: I cannot. I'll have to run them after merging, which is a bit of a shame. In the meantime I've confirmed that unit and type checks do pass.

SatyamMattoo commented 5 months ago

@josephjclark Sure, I'll look into these issues and include the integration tests for the same. Thankyou.

SatyamMattoo commented 5 months ago

Hello @josephjclark,

I've implemented the requested changes, which include:

  1. Implementation of the auto-install function within the handler.ts file.
  2. Renaming of the function to shouldAutoinstall.
  3. Removal of redundant logs.
  4. Addition of an integration test to verify the auto-installation of the adaptor ("common").

Please review these modifications at your convenience and inform me of any further adjustments or additions you deem necessary.

Best regards

SatyamMattoo commented 5 months ago

Hi @josephjclark,

Thanks for your detailed review! Your suggestions really helped improve the code quality. I've made all the requested changes. Please take a look whenever you get a chance. Let me know if there's anything else that needs improvement.

Best regards

josephjclark commented 5 months ago

This is great @SatyamMattoo, thank you for your patience!

josephjclark commented 5 months ago

@SatyamMattoo Your fix is now live in @openfn/cli@1.2.3 :tada: