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

downloader: Add the image URL into the error message. #3098

Merged
merged 1 commit into from
Mar 15, 2023

Conversation

OhmSpectator
Copy link
Member

In the case the URL of the image cannot be parsed for any reason, put it to the error message, so the user can see what can be the issue.

@rouming
Copy link
Contributor

rouming commented Mar 14, 2023

Almost good. ociRepositorySplit() on error path returns empty strings, so does not make any sense to print them.

In the case the URL of the image cannot be parsed for any reason, put it
to the error message, so the user can see what can be the issue.

Signed-off-by: Nikolay Martyanov <nikolay@zededa.com>
@OhmSpectator OhmSpectator force-pushed the feature/add-image-url-to-error branch from ec30fd6 to 88f1b7e Compare March 14, 2023 11:30
@OhmSpectator
Copy link
Member Author

Print only the error returned by the ociRepositorySplit function.
Add the original URL to the error message returned by the function.

Copy link
Contributor

@rouming rouming left a comment

Choose a reason for hiding this comment

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

Looks good.

Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

Looks good, but is the issue only for OCI images?
When we download from S3, Azure, https etc do we print the URL on failure?

@OhmSpectator
Copy link
Member Author

Looks good, but is the issue only for OCI images?
When we download from S3, Azure, https etc do we print the URL on failure?

I can check it if you want. But the bug I'm fixing in the scope of this PR is an error in the debug message on the OCI URLs handling (previously it printed an empty string). It's requested by the CS team, See EV-383 for the details.

@rouming
Copy link
Contributor

rouming commented Mar 15, 2023

This one PR is a request by the CS team: show the correct url on error path. I merge this since the change does exactly what was requested.

@rouming rouming merged commit f18cf06 into lf-edge:master Mar 15, 2023
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