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

add Check command to check if a component is compatible with edgee #198

Merged
merged 8 commits into from
Feb 5, 2025

Conversation

CLEMENTINATOR
Copy link
Contributor

Checklist

  • I have read the Contributor Guide
  • I have read and agree to the Code of Conduct
  • I have added a description of my changes and why I'd like them included in the section below

@KokaKiwi it is set to merge into your branch

@CLEMENTINATOR CLEMENTINATOR requested a review from a team as a code owner February 3, 2025 08:31
@CLEMENTINATOR
Copy link
Contributor Author

tested with rs component

╭─clement@brittle-hollow ~/work/components/example-rs-component ‹initial_import●›
╰─$ ../../edgee/target/debug/edgee components check
╭─clement@brittle-hollow ~/work/components/example-rs-component ‹initial_import●›
╰─$ ../../edgee/target/debug/edgee components check -c target/wasm32-wasip2/release/example_rs_component.wasm
Error: no exported instance named `edgee:protocols/consent-mapping`
╭─clement@brittle-hollow ~/work/components/example-rs-component ‹initial_import●›
╰─$ ../../edgee/target/debug/edgee components check -d target/wasm32-wasip2/release/example_rs_component.wasm                                                                                                           

@CLEMENTINATOR CLEMENTINATOR force-pushed the components-sanity-check branch from cdee835 to 073519d Compare February 3, 2025 08:35
@alexcasalboni
Copy link
Contributor

Imho, the Error: no exported instance named edgee:protocols/consent-mapping error is very confusing and we should try to wrap the runtime errors into understandable messages like "Component not found", "Component not valid", "Invalid component type", "Component is valid", etc.

Also, would it be possible to infer the component type (data collection, consent mapping, etc.) without specifying it with -c or -d? That would make it much easier to use.

@CLEMENTINATOR
Copy link
Contributor Author

Imho, the Error: no exported instance named edgee:protocols/consent-mapping error is very confusing and we should try to wrap the runtime errors into understandable messages like "Component not found", "Component not valid", "Invalid component type", "Component is valid", etc.

Also, would it be possible to infer the component type (data collection, consent mapping, etc.) without specifying it with -c or -d? That would make it much easier to use.

I'll wrap the errors !

Regarding the second question, I am not sure that we can detect the component type (directly from a wasm file i mean).

@alexcasalboni
Copy link
Contributor

alexcasalboni commented Feb 3, 2025

Regarding the second question, I am not sure that we can detect the component type (directly from a wasm file i mean).

We should be able to inspect the interface and check what it's export. Something like this:

wasm-tools component wit component.wasm

At some point the output should contain

package edgee:protocols {
    interface data-collection {
        ...
        page: func(e: event, cred: dict) -> result<edgee-request, string>;
        track: func(e: event, cred: dict) -> result<edgee-request, string>;
        user: func(e: event, cred: dict) -> result<edgee-request, string>;
    }
}

@CLEMENTINATOR
Copy link
Contributor Author

Regarding the second question, I am not sure that we can detect the component type (directly from a wasm file i mean).

We should be able to inspect the interface and check what it's export. Something like this:

wasm-tools component wit component.wasm

At some point the output should contain

package edgee:protocols {
    interface data-collection {
        ...
        page: func(e: event, cred: dict) -> result<edgee-request, string>;
        track: func(e: event, cred: dict) -> result<edgee-request, string>;
        user: func(e: event, cred: dict) -> result<edgee-request, string>;
    }
}

For me, this tool checks that the component implements a given world (and not the world that is in the component itself)

Eg if an user creates a cmp component, and defines it as a data-collection component in the registry, the tool should check the component against the data-collection component type and not the cmp component type (the one that is detected by wasm tools component wit)

@alexcasalboni
Copy link
Contributor

I think this command isn't going to be used by OSS developers too often and we probably want to automatically run it before edgee component push to make sure the .wasm file is valid before uploading it.

@CLEMENTINATOR
Copy link
Contributor Author

I think this command isn't going to be used by OSS developers too often and we probably want to automatically run it before edgee component push to make sure the .wasm file is valid before uploading it.

Exactly, its to precheck on the client side before we check it ourself on server side !

@KokaKiwi KokaKiwi force-pushed the feat/cli-component-commands branch from 73c413e to dee49cc Compare February 5, 2025 09:28
@CLEMENTINATOR CLEMENTINATOR force-pushed the components-sanity-check branch from e3b12da to 0947c3b Compare February 5, 2025 09:41
@CLEMENTINATOR CLEMENTINATOR merged commit e27b4cf into feat/cli-component-commands Feb 5, 2025
4 checks passed
@CLEMENTINATOR CLEMENTINATOR deleted the components-sanity-check branch February 5, 2025 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants