Add job log upload support #2447
Conversation
| { | ||
| return _resultsClient.UploadResultsJobLogAsync(planId, jobId, file, finalize, firstBlock, lineCount, cancellationToken: cancellationToken); | ||
| } | ||
| throw new InvalidOperationException("Results client is not initialized."); |
There was a problem hiding this comment.
Double checking this time to make sure the exception will be caught 😄
There was a problem hiding this comment.
Since we're dual-uploading, do we even need this exception? I wonder if we could just add some tracing here instead.
There was a problem hiding this comment.
We actually caught the exception before as well - the issue was in the handling part. 😓
Here we wanted to log a telemetry for all the failures, but on GHES server doesn't understand this record and errored out.
There was a problem hiding this comment.
Now we are properly guarded and if the feature flag is not on, nothing will be put on the Results processing queue.
I also tested these changes against a GHES instance.
There was a problem hiding this comment.
@brittanyellich we shouldn't get to this point if we aren't going to upload to results, so I feel the exception is still appropriate.
| } | ||
|
|
||
| // Constants specific to results | ||
| public static class Constants |
| { | ||
| return _resultsClient.UploadResultsJobLogAsync(planId, jobId, file, finalize, firstBlock, lineCount, cancellationToken: cancellationToken); | ||
| } | ||
| throw new InvalidOperationException("Results client is not initialized."); |
There was a problem hiding this comment.
Since we're dual-uploading, do we even need this exception? I wonder if we could just add some tracing here instead.
69b70db
* Refactor and add job log upload support * Rename method to be consistent
* Refactor and add job log upload support * Rename method to be consistent
Upload job logs to results and some refactoring to reduce duplicated code.