diff --git a/src/Tgstation.Server.Host/Components/Instance.cs b/src/Tgstation.Server.Host/Components/Instance.cs
index b431681550..77975923af 100644
--- a/src/Tgstation.Server.Host/Components/Instance.cs
+++ b/src/Tgstation.Server.Host/Components/Instance.cs
@@ -261,84 +261,111 @@ namespace Tgstation.Server.Host.Components
compileJob.Job = job;
- databaseContext.CompileJobs.Add(compileJob); // will be saved by job context
+ databaseContext.CompileJobs.Add(compileJob);
- job.PostComplete = async postCompleteCancellationToken =>
+ // The difficulty with compile jobs is they have a two part commit
+ await databaseContext.Save(cancellationToken).ConfigureAwait(false);
+ try
{
- await compileJobConsumer.LoadCompileJob(compileJob, postCompleteCancellationToken).ConfigureAwait(false);
- await eventConsumer.HandleEvent(EventType.DeploymentComplete, null, postCompleteCancellationToken).ConfigureAwait(false);
- };
-
- if (repositorySettings?.AccessToken != null)
+ await compileJobConsumer.LoadCompileJob(compileJob, cancellationToken).ConfigureAwait(false);
+ }
+ catch
{
- // potential for commenting on a test merge change
- var outgoingCompileJob = LatestCompileJob();
+ // So we need to un-commit the compile job if the above throws
+ databaseContext.CompileJobs.Remove(compileJob);
+ await databaseContext.Save(default).ConfigureAwait(false);
+ throw;
+ }
- if (outgoingCompileJob != null && outgoingCompileJob.RevisionInformation.CommitSha != compileJob.RevisionInformation.CommitSha && repositorySettings.PostTestMergeComment.Value)
+ await eventConsumer.HandleEvent(EventType.DeploymentComplete, null, cancellationToken).ConfigureAwait(false);
+
+ await PostDeploymentComments(compileJob, repositorySettings, repoOwner, repoName).ConfigureAwait(false);
+ }
+
+ ///
+ /// Post deployment GitHub comments.
+ ///
+ /// The deployed .
+ /// The .
+ /// The GitHub repostiory owner.
+ /// The GitHub repostiory name.
+ /// A representing the running operation.
+ async Task PostDeploymentComments(
+ CompileJob compileJob,
+ RepositorySettings repositorySettings,
+ string repoOwner,
+ string repoName)
+ {
+ if (repositorySettings?.AccessToken == null)
+ return;
+
+ // potential for commenting on a test merge change
+ var outgoingCompileJob = LatestCompileJob();
+
+ if (outgoingCompileJob == null || outgoingCompileJob.RevisionInformation.CommitSha == compileJob.RevisionInformation.CommitSha || !repositorySettings.PostTestMergeComment.Value)
+ return;
+
+ var gitHubClient = gitHubClientFactory.CreateClient(repositorySettings.AccessToken);
+
+ async Task CommentOnPR(int prNumber, string comment)
+ {
+ try
{
- var gitHubClient = gitHubClientFactory.CreateClient(repositorySettings.AccessToken);
-
- async Task CommentOnPR(int prNumber, string comment)
- {
- try
- {
- await gitHubClient.Issue.Comment.Create(repoOwner, repoName, prNumber, comment).ConfigureAwait(false);
- }
- catch (ApiException e)
- {
- logger.LogWarning("Error posting GitHub comment! Exception: {0}", e);
- }
- }
-
- var tasks = new List();
-
- string FormatTestMerge(TestMerge testMerge, bool updated) => String.Format(CultureInfo.InvariantCulture, "#### Test Merge {4}{0}{0}##### Server Instance{0}{5}{1}{0}{0}##### Revision{0}Origin: {6}{0}Pull Request: {2}{0}Server: {7}{3}",
- Environment.NewLine,
- repositorySettings.ShowTestMergeCommitters.Value ? String.Format(CultureInfo.InvariantCulture, "{0}{0}##### Merged By{0}{1}", Environment.NewLine, testMerge.MergedBy.Name) : String.Empty,
- testMerge.PullRequestRevision,
- testMerge.Comment != null ? String.Format(CultureInfo.InvariantCulture, "{0}{0}##### Comment{0}{1}", Environment.NewLine, testMerge.Comment) : String.Empty,
- updated ? "Updated" : "Deployed",
- metadata.Name,
- compileJob.RevisionInformation.OriginCommitSha,
- compileJob.RevisionInformation.CommitSha);
-
- // added prs
- foreach (var I in compileJob
- .RevisionInformation
- .ActiveTestMerges
- .Select(x => x.TestMerge)
- .Where(x => !outgoingCompileJob
- .RevisionInformation
- .ActiveTestMerges
- .Any(y => y.TestMerge.Number == x.Number)))
- tasks.Add(CommentOnPR(I.Number, FormatTestMerge(I, false)));
-
- // removed prs
- foreach (var I in outgoingCompileJob
- .RevisionInformation
- .ActiveTestMerges
- .Select(x => x.TestMerge)
- .Where(x => !compileJob
- .RevisionInformation
- .ActiveTestMerges
- .Any(y => y.TestMerge.Number == x.Number)))
- tasks.Add(CommentOnPR(I.Number, "#### Test Merge Removed"));
-
- // updated prs
- foreach (var I in compileJob
- .RevisionInformation
- .ActiveTestMerges
- .Select(x => x.TestMerge)
- .Where(x => outgoingCompileJob
- .RevisionInformation
- .ActiveTestMerges
- .Any(y => y.TestMerge.Number == x.Number)))
- tasks.Add(CommentOnPR(I.Number, FormatTestMerge(I, true)));
-
- if (tasks.Any())
- await Task.WhenAll(tasks).ConfigureAwait(false);
+ await gitHubClient.Issue.Comment.Create(repoOwner, repoName, prNumber, comment).ConfigureAwait(false);
+ }
+ catch (ApiException e)
+ {
+ logger.LogWarning("Error posting GitHub comment! Exception: {0}", e);
}
}
+
+ var tasks = new List();
+
+ string FormatTestMerge(TestMerge testMerge, bool updated) => String.Format(CultureInfo.InvariantCulture, "#### Test Merge {4}{0}{0}##### Server Instance{0}{5}{1}{0}{0}##### Revision{0}Origin: {6}{0}Pull Request: {2}{0}Server: {7}{3}",
+ Environment.NewLine,
+ repositorySettings.ShowTestMergeCommitters.Value ? String.Format(CultureInfo.InvariantCulture, "{0}{0}##### Merged By{0}{1}", Environment.NewLine, testMerge.MergedBy.Name) : String.Empty,
+ testMerge.PullRequestRevision,
+ testMerge.Comment != null ? String.Format(CultureInfo.InvariantCulture, "{0}{0}##### Comment{0}{1}", Environment.NewLine, testMerge.Comment) : String.Empty,
+ updated ? "Updated" : "Deployed",
+ metadata.Name,
+ compileJob.RevisionInformation.OriginCommitSha,
+ compileJob.RevisionInformation.CommitSha);
+
+ // added prs
+ foreach (var I in compileJob
+ .RevisionInformation
+ .ActiveTestMerges
+ .Select(x => x.TestMerge)
+ .Where(x => !outgoingCompileJob
+ .RevisionInformation
+ .ActiveTestMerges
+ .Any(y => y.TestMerge.Number == x.Number)))
+ tasks.Add(CommentOnPR(I.Number, FormatTestMerge(I, false)));
+
+ // removed prs
+ foreach (var I in outgoingCompileJob
+ .RevisionInformation
+ .ActiveTestMerges
+ .Select(x => x.TestMerge)
+ .Where(x => !compileJob
+ .RevisionInformation
+ .ActiveTestMerges
+ .Any(y => y.TestMerge.Number == x.Number)))
+ tasks.Add(CommentOnPR(I.Number, "#### Test Merge Removed"));
+
+ // updated prs
+ foreach (var I in compileJob
+ .RevisionInformation
+ .ActiveTestMerges
+ .Select(x => x.TestMerge)
+ .Where(x => outgoingCompileJob
+ .RevisionInformation
+ .ActiveTestMerges
+ .Any(y => y.TestMerge.Number == x.Number)))
+ tasks.Add(CommentOnPR(I.Number, FormatTestMerge(I, true)));
+
+ if (tasks.Any())
+ await Task.WhenAll(tasks).ConfigureAwait(false);
}
///
diff --git a/src/Tgstation.Server.Host/Jobs/JobManager.cs b/src/Tgstation.Server.Host/Jobs/JobManager.cs
index a3919c8dd2..20e97c3e1d 100644
--- a/src/Tgstation.Server.Host/Jobs/JobManager.cs
+++ b/src/Tgstation.Server.Host/Jobs/JobManager.cs
@@ -81,43 +81,8 @@ namespace Tgstation.Server.Host.Jobs
{
await databaseContextFactory.UseContext(async databaseContext =>
{
- async Task HandleExceptions(Task task)
- {
- void LogRegularException() => logger.LogDebug("Job {0} exited with error! Exception: {1}", job.Id, job.ExceptionDetails);
- try
- {
- await task.ConfigureAwait(false);
- }
- catch (OperationCanceledException)
- {
- logger.LogDebug("Job {0} cancelled!", job.Id);
- job.Cancelled = true;
- }
- catch (JobException e)
- {
- job.ErrorCode = e.ErrorCode;
- job.ExceptionDetails = e.Message;
- LogRegularException();
- if (e.InnerException != null)
- logger.LogDebug(
- "Inner exception for job {0}: {1}",
- job.Id,
- e.InnerException is JobException
- ? e.InnerException.Message
- : e.InnerException.ToString());
- }
- catch (Exception e)
- {
- job.ExceptionDetails = e.ToString();
- LogRegularException();
- }
- finally
- {
- job.StoppedAt = DateTimeOffset.Now;
- }
- }
-
- async Task RunJobInternal()
+ void LogRegularException() => logger.LogDebug("Job {0} exited with error! Exception: {1}", job.Id, job.ExceptionDetails);
+ try
{
var oldJob = job;
job = new Job { Id = oldJob.Id };
@@ -127,20 +92,35 @@ namespace Tgstation.Server.Host.Jobs
logger.LogDebug("Job {0} completed!", job.Id);
}
-
- await HandleExceptions(RunJobInternal()).ConfigureAwait(false);
+ catch (OperationCanceledException)
+ {
+ logger.LogDebug("Job {0} cancelled!", job.Id);
+ job.Cancelled = true;
+ }
+ catch (JobException e)
+ {
+ job.ErrorCode = e.ErrorCode;
+ job.ExceptionDetails = e.Message;
+ LogRegularException();
+ if (e.InnerException != null)
+ logger.LogDebug(
+ "Inner exception for job {0}: {1}",
+ job.Id,
+ e.InnerException is JobException
+ ? e.InnerException.Message
+ : e.InnerException.ToString());
+ }
+ catch (Exception e)
+ {
+ job.ExceptionDetails = e.ToString();
+ LogRegularException();
+ }
+ finally
+ {
+ job.StoppedAt = DateTimeOffset.Now;
+ }
await databaseContext.Save(default).ConfigureAwait(false);
-
- bool JobErroredOrCancelled() => job.ExceptionDetails != null || job.Cancelled == true;
-
- // ok so, now it's time for the post commit step if it exists
- if (!JobErroredOrCancelled() && job.PostComplete != null)
- {
- await HandleExceptions(job.PostComplete(cancellationToken)).ConfigureAwait(false);
- if (JobErroredOrCancelled())
- await databaseContext.Save(default).ConfigureAwait(false);
- }
}).ConfigureAwait(false);
}
finally
diff --git a/src/Tgstation.Server.Host/Models/Job.cs b/src/Tgstation.Server.Host/Models/Job.cs
index 7ff111762c..134993f14c 100644
--- a/src/Tgstation.Server.Host/Models/Job.cs
+++ b/src/Tgstation.Server.Host/Models/Job.cs
@@ -1,8 +1,4 @@
-using System;
-using System.ComponentModel.DataAnnotations;
-using System.ComponentModel.DataAnnotations.Schema;
-using System.Threading;
-using System.Threading.Tasks;
+using System.ComponentModel.DataAnnotations;
namespace Tgstation.Server.Host.Models
{
@@ -26,13 +22,6 @@ namespace Tgstation.Server.Host.Models
[Required]
public Instance Instance { get; set; }
- ///
- /// A to run after the job completes. This will not affect the time, unless it is cancelled or errors
- ///
- /// This should only be used where there are database dependencies that also rely on the Job itself completing A.K.A. manually initiated s
- [NotMapped]
- public Func PostComplete { get; set; }
-
///
/// Convert the to it's API form
///