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

Better error if component from registry has regrettable version #2634

Merged
merged 1 commit into from
Jul 14, 2024

Conversation

itowlson
Copy link
Contributor

If you have a component sourced from a registry, the version has to be semver. The previous error message was bad.

Before:

ivan@hestia:~/testing/wkgtest$ spin up
Error: TOML parse error at line 15, column 10
   |
15 | source = { registry = "registrytest-vfztdiyy.fermyon.app", package = "component:ccspintest", version = "canary" }
   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
data did not match any variant of untagged enum ComponentSource

After:

ivan@hestia:~/testing/wkgtest$ spin up
Error: Failed to load manifest from "spin.toml"

Caused by:
   0: Failed to load Spin app from "spin.toml"
   1: Failed to load component `wkgtest`
   2: Failed to load Wasm source "component:ccspintest@canary" from registry Registry(registrytest-vfztdiyy.fermyon.app)
   3: Component wkgtest specifies an invalid semantic version ("canary") for its package version
   4: unexpected character 'c' while parsing major version number

version: semver::Version,
version: String,
Copy link
Collaborator

@lann lann Jul 12, 2024

Choose a reason for hiding this comment

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

Another option here would be a custom deserialize_with; the thing I appreciate about using Version here is that you get nice location information in errors.

NEVER MIND I GET IT NOW

@itowlson itowlson merged commit 66ede35 into fermyon:main Jul 14, 2024
17 checks passed
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