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

fix(types): warn on unknown interface #536

Merged

Conversation

brooksmtownsend
Copy link
Member

Feature or Problem

This PR downgrades the unrecognized interface error into a warning as inspired by some of the recent additions to keyvalue like batch and watch. We want to maintain a balance in validation to make sure that users don't accidentally link on, say, wasi:keyvalue/atormics, but we don't completely block the addition of new interfaces e.g. in wasmcloud:[email protected]

Open to thoughts here.

Related Issues

Release Information

Consumer Impact

Testing

Unit Test(s)

Acceptance or Integration

Manual Verification

@brooksmtownsend brooksmtownsend requested a review from a team as a code owner January 2, 2025 16:44
Copy link
Member

@lxfontes lxfontes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our interface map is not the authoritative source of all valid interfaces, but hey it is pretty good!

I wonder if this is where --force comes in.
The safest path is to fail on any validation error, even warnings. Ex:

wash app deploy newish-interface.yaml
Warning: Unknown interface "bla"
exit 1

vs

wash app deploy --force newish-interface.yaml
Warning: Unknown interface "bla"
exit 0

@brooksmtownsend
Copy link
Member Author

I like that idea @lxfontes , I think we should fail on warnings and then allow forcing past that. My thought is that we should still fail without allowing a --force on the actual errors, e.g. an invalid manifest or a link to a component that doesn't exist. Do you think a --force should override all of that or still have some failure cases?

@lxfontes
Copy link
Member

lxfontes commented Jan 2, 2025

agreed 👍 --force should only bypass warnings

@brooksmtownsend brooksmtownsend merged commit 77f5bc8 into wasmCloud:main Jan 3, 2025
5 checks passed
@brooksmtownsend brooksmtownsend deleted the fix/warn-on-unknown-interface branch January 3, 2025 21:30
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.

3 participants