Introduce versioning into the Java SDK #224
Conversation
halspang
force-pushed
the
halspang/java_versioning
branch
3 times, most recently
from
6df61ee to
c65b05e
Compare
halspang
force-pushed
the
halspang/java_versioning
branch
from
c65b05e to
acc7e56
Compare
halspang
force-pushed
the
halspang/java_versioning
branch
from
acc7e56 to
5bdebb8
Compare
| .findFirst() | ||
| .orElse(null); | ||
|
|
||
| logger.log(Level.INFO, String.format("Version from orchestration events: '%s'", version)); |
There was a problem hiding this comment.
This log statement will be written for every orchestration work item, which I think will be too noisy. Let's log it only if version is not null. Alternatively, if you care about the case where version is not specified, then we should log this only when we find an EXECUTIONSTARTED event in the new event list.
There was a problem hiding this comment.
I think this actually is just leftover debugging. I'm going to remove it, let me know if you think having it under those cases is important and I can add it specifically for that.
client/src/main/java/com/microsoft/durabletask/DurableTaskGrpcWorker.java
Show resolved
Hide resolved
client/src/main/java/com/microsoft/durabletask/util/VersionUtils.java
Outdated
Show resolved
Hide resolved
client/src/test/java/com/microsoft/durabletask/IntegrationTests.java
Outdated
Show resolved
Hide resolved
halspang
force-pushed
the
halspang/java_versioning
branch
from
5bdebb8 to
30b53eb
Compare
This commit adds the versioning feature from the dotnet SDK (durabletask-dotnet). This allows for the version to be passed via the context and for the worker to reject/fail orchestrations based on the provided version. Signed-off-by: Hal Spang <[email protected]>
halspang
force-pushed
the
halspang/java_versioning
branch
from
30b53eb to
6863dcc
Compare
Signed-off-by: Hal Spang <[email protected]>
| @@ -379,6 +407,16 @@ public <V> Task<V> callSubOrchestrator( | |||
| } | |||
| createSubOrchestrationActionBuilder.setInstanceId(instanceId); | |||
|
|
|||
| if (options instanceof NewSubOrchestrationInstanceOptions && ((NewSubOrchestrationInstanceOptions)options).getVersion() != null) { | |||
| NewSubOrchestrationInstanceOptions subOrchestrationOptions = (NewSubOrchestrationInstanceOptions) options; | |||
| if (subOrchestrationOptions.getVersion() != null) { | |||
There was a problem hiding this comment.
Is this if-check redundant with the check on line 410?
There was a problem hiding this comment.
Yes, it sure was. I originally had it just there as I had it doing a .NET style type matching thing but then I found out that isn't supported through all the Java versions we have so the check got split up. Fixed.
| } | ||
| } else if (num1 != null) { | ||
| // part1 is numeric, part2 is not - numeric versions come before non-numeric | ||
| return 1; |
There was a problem hiding this comment.
Per our offline discussion, I'm still concerned that this error handling behavior is different from the behavior we have in the .NET case, where anything that doesn't match x.y.z is immediately reverts to string comparison logic.
There was a problem hiding this comment.
Updated to match the .NET style.
Signed-off-by: Hal Spang <[email protected]>
halspang
force-pushed
the
halspang/java_versioning
branch
from
c435427 to
b450f9e
Compare
Issue describing the changes in this PR
This commit adds the versioning feature from the dotnet SDK (durabletask-dotnet). This allows for the version to be passed via the context and for the worker to reject/fail orchestrations based on the provided version.
Pull request checklist
CHANGELOG.mdAdditional information
Additional PR information