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: logs to have same current model and extracted model #11118

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

Jougan-0
Copy link
Contributor

@Jougan-0 Jougan-0 commented Jun 5, 2024

Notes for Reviewers

This PR fixes #11117
The model name for a model whose components were failing were missing and the error couldn't be located for which model it happened so whenever error happens while generating components log the component name before the error.
image

Signed commits

  • Yes, I signed my commits.

…signoff

Signed-off-by: Jougan-0 <prasantmishra2018@gmail.com>
@github-actions github-actions bot added the component/mesheryctl CLI for Meshery label Jun 5, 2024
@Jougan-0 Jougan-0 requested a review from MUzairS15 June 5, 2024 12:58
Copy link

github-actions bot commented Jun 5, 2024

Copy link

codecov bot commented Jun 5, 2024

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 9.14%. Comparing base (63478b2) to head (63e1958).
Report is 178 commits behind head on master.

Files Patch % Lines
mesheryctl/internal/cli/root/registry/error.go 0.00% 1 Missing ⚠️
mesheryctl/internal/cli/root/registry/generate.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11118      +/-   ##
==========================================
- Coverage    9.15%    9.14%   -0.01%     
==========================================
  Files         146      146              
  Lines       19243    19247       +4     
==========================================
  Hits         1761     1761              
- Misses      17180    17184       +4     
  Partials      302      302              
Flag Coverage Δ
unittests 9.14% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -19,7 +19,7 @@ func ErrUpdateRegistry(err error, path string) error {
}

func ErrGenerateModel(err error, modelName string) error {
return errors.New(ErrGenerateModelCode, errors.Alert, []string{fmt.Sprintf("error generating model: %s", modelName)}, []string{err.Error()}, []string{"Registrant used for the model is not supported", "Verify the model's source URL.", "Failed to create a local directory in the filesystem for this model."}, []string{"Ensure that each kind of registrant used is a supported kind.", "Ensure correct model source URL is provided and properly formatted.", "Ensure sufficient permissions to allow creation of model directory."})
return errors.New(ErrGenerateModelCode, errors.Alert, []string{fmt.Sprintf("error generating model: %s", modelName)}, []string{fmt.Sprintf("Error generating model: %s\n %s", modelName, err.Error())}, []string{"Registrant used for the model is not supported", "Verify the model's source URL.", "Failed to create a local directory in the filesystem for this model."}, []string{"Ensure that each kind of registrant used is a supported kind.", "Ensure correct model source URL is provided and properly formatted.", "Ensure sufficient permissions to allow creation of model directory."})
Copy link
Contributor

Choose a reason for hiding this comment

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

fmt.Sprintf("Error generating model: %s\n %s")
This is being repeated, it is already present in short description

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But the logging done in the RegistryGenerate file it writes only the longDescription so either we can use it to write short + long or we can just put in long and remove from short.
Thoughts?

@Jougan-0 Jougan-0 changed the title fix logs to have same current model and extracted model under same w… fix: logs to have same current model and extracted model Jun 5, 2024
@MUzairS15 MUzairS15 merged commit 29593c9 into meshery:master Jun 13, 2024
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/mesheryctl CLI for Meshery
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[mesheryctl] Mismatch in position of current model and extracted model in logs.
2 participants