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 errors to enhance model import error logging #681

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

riyaa14
Copy link
Contributor

@riyaa14 riyaa14 commented Feb 13, 2025

Description

This PR fixes Meshery #13574

Notes for Reviewers
Added new errors to meshkit/files/errors.go, that are useful for error logging while processing model artifacts. Currently, file errors defined here are specific to design import, we need to extend error messages to support better error logging in model import.

Signed commits

  • Yes, I signed my commits.

@riyaa14 riyaa14 force-pushed the model-import-errors branch from 3b69606 to c30c5a9 Compare February 16, 2025 21:52
@@ -32,9 +32,9 @@ var ValidIacExtensions = map[string]bool{
".tar": true,
".tar.gz": true,
".tar.tgz": true,
".zip": true,
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we removing zip files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed that zip isn't working for both designs and models, this can confuse the user if we say zip is supported. I confirmed this on slack here

Copy link
Member

Choose a reason for hiding this comment

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

dont remove the zip support here , we do support zips in places ( it is just that we dont support it for all type ) .

Copy link
Contributor Author

@riyaa14 riyaa14 Feb 24, 2025

Choose a reason for hiding this comment

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

@aabidsofi19 got it, added it back to common valid extensions and specifically removing it for model import error - Line 371

files/error.go Outdated

remedy := []string{
"Make sure the archive is OCI compliant. Meshery Models should be OCI compliant archives.",
"Try using a different extraction tool or library to rule out compatibility issues.",
Copy link
Member

Choose a reason for hiding this comment

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

Users are supposed to use a library?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my bad, this error message was close to ErrFailedToExtractArchive, so I copied its error message here.
I have improved the error message now ✅

files/error.go Outdated
}

remedy := []string{
"Review the YAML syntax in the file and correct any errors.",
"Use a YAML validator or linter to identify and fix issues.",
"Ensure the file adheres to the YAML specification.",
"Ensure the YAML conforms to the Meshery model schema. You can refer to the Meshery model schema documentation for more details.",
Copy link
Member

Choose a reason for hiding this comment

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

Please provide a hyperlink to schema or docs.

Copy link
Member

Choose a reason for hiding this comment

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

Or to a troubleshooting guide on the subject.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

provided a link to 'contributing to models' ✅

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.

[Server] Enhance error logging in model import
4 participants