Update Core #376
Update Core #376
Conversation
Makes it easier to run this without local tooling, which is a massive pain on linux
With temp workaround for job ordering
Sushisource
force-pushed
the
core-update
branch
from
fc1e49e to
18e7bcc
Compare
Sushisource
force-pushed
the
core-update
branch
from
045dbce to
95428ea
Compare
| var isReplayingValues = await Env.Client.GetWorkflowHandle<MiscHelpersWorkflow>( | ||
| workflowId).QueryAsync(wf => wf.GetEventsForIsReplaying()); | ||
| Assert.Equal(new[] { false, true }, isReplayingValues); | ||
| Assert.Equal(new[] { false, true, true }, isReplayingValues); |
There was a problem hiding this comment.
This is also due to the evict-on-finish change
There was a problem hiding this comment.
Took me a sec to grok what was happening here. But now I see that on line 1214 above we issue a post-complete query which now causes full replay where it didn't before
Sushisource
force-pushed
the
core-update
branch
from
95428ea to
6b33906
Compare
| // We must set the sync context to null so work isn't posted there | ||
| SynchronizationContext.SetSynchronizationContext(null); | ||
| // TODO: Temporary workaround in lieu of https://github.com/temporalio/sdk-dotnet/issues/375 | ||
| var sortedJobs = act.Jobs.OrderBy(j => |
There was a problem hiding this comment.
I am a bit surprised we didn't order before. But I assume this order by matches previous Core versions before this update? Meaning we feel fairly confident that replays will work the same on .NET SDK upgrade here?
There was a problem hiding this comment.
Yes, this doesn't change anything. It matches the previous Core ordering
| { | ||
| Seq = seq, | ||
| StartToFireTimeout = Google.Protobuf.WellKnownTypes.Duration.FromTimeSpan(delay), | ||
| Summary = instance.PayloadConverter.ToPayload(string.Empty), |
There was a problem hiding this comment.
| Summary = instance.PayloadConverter.ToPayload(string.Empty), |
I think we want this to be null instead of a full payload (and we will add if summary is present as part of #359). Is there some kind of check Core-side forcing us to set this?
There was a problem hiding this comment.
No, I was confused by something going on in one of the tests and forgot to clean this up
| var isReplayingValues = await Env.Client.GetWorkflowHandle<MiscHelpersWorkflow>( | ||
| workflowId).QueryAsync(wf => wf.GetEventsForIsReplaying()); | ||
| Assert.Equal(new[] { false, true }, isReplayingValues); | ||
| Assert.Equal(new[] { false, true, true }, isReplayingValues); |
There was a problem hiding this comment.
Took me a sec to grok what was happening here. But now I see that on line 1214 above we issue a post-complete query which now causes full replay where it didn't before
2 participants
Updating core to separate those changes from the PR for slot suppliers #372
Which I will rebase after this is merged