This repository has been archived by the owner on Jul 30, 2024. It is now read-only.
FailValidationWithMixedSymbols #656
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cristinamanum
requested review from
shishirx34,
chenriksson,
scottbommarito,
zhhyu,
loic-sharma,
agr,
joelverhagen and
skofman1
chenriksson
approved these changes
Nov 7, 2018
| @@ -38,6 +38,7 @@ public interface ITelemetryService : ISubscriptionProcessorTelemetryService | |||
| /// <param name="packageNormalizedVersion">The package normalized version.</param> | |||
| /// <param name="validationStatus">The validation result.</param> | |||
| /// <param name="issue">Information about the issue id failed or empty if passed..</param> | |||
| void TrackSymbolsValidationResultEvent(string packageId, string packageNormalizedVersion, ValidationStatus validationStatus, string issue); | |||
| /// <param name="assemblyName">The assembly name.</param> | |||
| void TrackSymbolsValidationResultEvent(string packageId, string packageNormalizedVersion, ValidationStatus validationStatus, string issue, string assemblyName); | |||
There was a problem hiding this comment.
nit: assemblyName is really assemblyPath?
There was a problem hiding this comment.
It is the assembly name. The log looks like:
{"PackageNormalizedVersion":"1.0.0","PackageId":"SymServBadPortableCM1","ValidationResult":"Succeeded","AssemblyName":"PortableTestAppCM11.dll"}
| @@ -64,7 +64,7 @@ public async Task<IValidationResult> ValidateSymbolsAsync(SymbolsValidatorMessag | |||
| { | |||
| if (!SymbolsHaveMatchingPEFiles(pdbs, pes)) | |||
| { | |||
| _telemetryService.TrackSymbolsValidationResultEvent(message.PackageId, message.PackageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound)); | |||
| _telemetryService.TrackSymbolsValidationResultEvent(message.PackageId, message.PackageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound), assemblyName:""); | |||
There was a problem hiding this comment.
nit: why not use PDB path instead of assemblyName... in this scenario, you could point the user at the PDB, right?
There was a problem hiding this comment.
Updated to identify the missing assembly.
shishirx34
approved these changes
Nov 7, 2018
joelverhagen
pushed a commit
that referenced
this pull request
Sep 27, 2019
* Extracting icon URL from catalog pages. * Moved the ServicePointManager.DefaultConnectionLimit to the job class. * Error handling and timeouts. * Retries and icon URL validation. * Initial cache implementation. * Common retrying code. Actually copying blob when found in cache. * Processeing HTTP 308 resopnse manually. * Better TLS failure check * CallGuid and minor exception update. * Organized usings. Const for blob storage copy attempts. * Using CatalogClient to get catalog leaves. * Not using HttpClient directly to retrieve the icon data. * logging deletes * Removed unused arguments * Ignoring and logging unexpected responses instead of throwing. * Extracted catalog data processing into its own class. * Extracted external content provider. * Cache in a separate class. * Logging update. * Handling copy from cache/storage failures. * Custom JsonConverter to be able to read certain funny catalog leaves. * Trying to warn instead of erroring for missing cache item. * Changed the implementation to pick the first element of the array instead of concatenating (to align the behavior with similar code). Renamed classes accordingly. * Retry should actually throw on last attempt. * LogWarning -> LogError, to get errors to AI * Proper package filename for the embedded icon case. * Putting back LogWarning for non-important exceptions * Logging reported content type for analysis. * Logging storageUrl when copy from cache fails. * More tests. * whitespace * Minor feedback addressed. * Storage -> IStorage where possible. More tests. * Minor feedback. * Separate HttpMessageHandler for collector to avoid disposing of the shared instance. * Documentation. * catalog2icon package * build script update * Logging leaf retreival exceptions. * logging offending leaf URL * IHttpClient & HttpClientWrapper. * Refactored `ProcessExternalIconUrlAsync` * ToLower -> ToLowerInvariant * UTs for cache. * content type fix
joelverhagen
pushed a commit
that referenced
this pull request
Oct 26, 2020
* Fix mixed(failing and passing) symbols validation. * Feedback - add better telemetry.
joelverhagen
pushed a commit
that referenced
this pull request
Oct 26, 2020
* Extracting icon URL from catalog pages. * Moved the ServicePointManager.DefaultConnectionLimit to the job class. * Error handling and timeouts. * Retries and icon URL validation. * Initial cache implementation. * Common retrying code. Actually copying blob when found in cache. * Processeing HTTP 308 resopnse manually. * Better TLS failure check * CallGuid and minor exception update. * Organized usings. Const for blob storage copy attempts. * Using CatalogClient to get catalog leaves. * Not using HttpClient directly to retrieve the icon data. * logging deletes * Removed unused arguments * Ignoring and logging unexpected responses instead of throwing. * Extracted catalog data processing into its own class. * Extracted external content provider. * Cache in a separate class. * Logging update. * Handling copy from cache/storage failures. * Custom JsonConverter to be able to read certain funny catalog leaves. * Trying to warn instead of erroring for missing cache item. * Changed the implementation to pick the first element of the array instead of concatenating (to align the behavior with similar code). Renamed classes accordingly. * Retry should actually throw on last attempt. * LogWarning -> LogError, to get errors to AI * Proper package filename for the embedded icon case. * Putting back LogWarning for non-important exceptions * Logging reported content type for analysis. * Logging storageUrl when copy from cache fails. * More tests. * whitespace * Minor feedback addressed. * Storage -> IStorage where possible. More tests. * Minor feedback. * Separate HttpMessageHandler for collector to avoid disposing of the shared instance. * Documentation. * catalog2icon package * build script update * Logging leaf retreival exceptions. * logging offending leaf URL * IHttpClient & HttpClientWrapper. * Refactored `ProcessExternalIconUrlAsync` * ToLower -> ToLowerInvariant * UTs for cache. * content type fix
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix the case when a symbol package has both symbols that pass validation and symbols that fail validation.
The validation should fail in this case.