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

make AppConfig.name optional in wasmer-config #5305

Merged
merged 4 commits into from
Dec 17, 2024
Merged

Conversation

ayys
Copy link
Member

@ayys ayys commented Dec 13, 2024

The wasmer-cli treats AppConfig objects without names as valid, but
this behavior is not reflected in the AppConfig schema.

This discrepancy caused issues when attempting to parse a "valid"
AppConfig as per the wasmer cli. Wasmer CLI injects a name
if it does not exist, so this issue is not seen. But as the name field was
marked as required, other clients using wasmer-config would fail.

Example app.yaml config that works with wasmer-cli but not with other clients:

kind: wasmer.io/App.v0
package: wasmer/hello

This commit makes the AppConfig.name field optional, thereby allowing
programs to successfully parse configurations without a name field.

The wasmer-cli treats `AppConfig` objects without names as valid, but
this behavior is not reflected in the `AppConfig` schema.

This discrepancy caused issues when attempting to parse a "valid"
`AppConfig` using the wasmer-config create command, as the name field
was marked as required.

This commit makes the `AppConfig.name` field optional, thereby allowing
programs to successfully parse configurations without a name field.
@ayys ayys requested a review from syrusakbary as a code owner December 13, 2024 15:50
@ayys ayys marked this pull request as draft December 16, 2024 08:15
@ayys ayys marked this pull request as ready for review December 17, 2024 08:39
@maminrayej maminrayej merged commit 5708ca8 into main Dec 17, 2024
73 checks passed
@maminrayej maminrayej deleted the make-appname-optional branch December 17, 2024 11:03
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