-
Notifications
You must be signed in to change notification settings - Fork 45
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
Improve warning message with no digest found #493
Conversation
Currently the message only mentions the artifact to download but not where it originates from, this makes it very hard to analyze and fix the issue (actually one needs to to run a debugger). This enhances the message a tiny little bit to mention the artifact descriptors repository URL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks fine.
We should consider if we can improve this to not log for local transfers, but this additional information should help make it clear when this is happening.
@@ -491,7 +491,8 @@ private OutputStream addPreSteps(ProcessingStepHandler handler, IArtifactDescrip | |||
addChecksumVerifiers(descriptor, downloadChecksumSteps, skipChecksums, IArtifactDescriptor.DOWNLOAD_CHECKSUM); | |||
if (downloadChecksumSteps.isEmpty() && !isLocal()) { | |||
LogHelper.log(new Status(IStatus.WARNING, Activator.ID, | |||
NLS.bind(Messages.noDigestAlgorithmToVerifyDownload, descriptor.getArtifactKey()))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notice that this call is guarded by `!isLocal()" because of course if the repository is local, this is not a download.
@@ -94,7 +94,8 @@ protected IStatus getArtifact(IArtifactDescriptor artifactDescriptor, OutputStre | |||
IArtifactDescriptor.DOWNLOAD_CHECKSUM, Collections.emptySet()); | |||
if (steps.isEmpty()) { | |||
LogHelper.log(new Status(IStatus.WARNING, Activator.ID, | |||
NLS.bind(Messages.noDigestAlgorithmToVerifyDownload, artifactDescriptor.getArtifactKey()))); | |||
NLS.bind(Messages.noDigestAlgorithmToVerifyDownload, artifactDescriptor.getArtifactKey(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was hoping that we could avoid calling this if the artifact is local. See comment below...
I think the reason is because RawMirrorRequest is always against a remote, at least while debugging it really tries to download from a remote. |
Currently the message only mentions the artifact to download but not where it originates from, this makes it very hard to analyze and fix the issue (actually one needs to to run a debugger).
This enhances the message a tiny little bit to mention the artifact descriptors repository URL.