Skip to content
This repository has been archived by the owner on Sep 27, 2023. It is now read-only.

feat: add CLI + Pipeline compatibility check #63

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

idanmuze
Copy link

@idanmuze idanmuze commented May 6, 2023

This feature gives a helpful message when running the CLI to clarify whether the CLI and the slated pipeline are compatible.

cicada_issue_2_image_b

cicada_issue_2_image_a

I ran into an issue while working on this, however.

I failed to run any pipeline that specified a Cicada version that wasn't the same as the CLI's one.
For example, I'm running the latest CLI version, 0.1.50.

If I change the Cicada import in .cicada/test.ts from

import { Job, Pipeline } from "https://deno.land/x/cicada/mod.ts"; (which will resolve to the latest version, 0.1.50)

to

import { Job, Pipeline } from "https://deno.land/x/[email protected]/mod.ts";

I get the following error:

Error: Module does not export a Pipeline or Image
[error] Failed to run deno script

It is serialize.ts that is throwing the error.:
cicada_issue_10_serialize_error_thrower_alt

This is confusing. test.ts still exports the default type pipeline (the only line I changed in the pipeline is the Cicada import).

Though I'm not sure what the cause is, it likely is just the method Cicada currently uses to cache the module. It may be that different Cicada versions of Pipeline are being treated as entirely different types. This causes them not to make it passed serialize.ts.

I didn't want to make non-trivial changes to the caching code before getting feedback on whether I'm on the right track, so here is a draft.

@mschrage mschrage requested a review from grant0417 May 8, 2023 16:36
@grant0417
Copy link
Contributor

Sorry for the delay on the review here, it all looks good to me, the only change would be not actually grabbing the latest version and just say something like "using latest version of definitions"

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants