save queue name into Attempts table #17921
Conversation
github-actions
bot
added
area/api
...sistence/job-persistence/src/main/java/io/airbyte/persistence/job/DefaultJobPersistence.java
Show resolved
Hide resolved
| @@ -18,6 +18,7 @@ public interface ReplicationActivity { | |||
| StandardSyncOutput replicate(JobRunConfig jobRunConfig, | |||
| IntegrationLauncherConfig sourceLauncherConfig, | |||
| IntegrationLauncherConfig destinationLauncherConfig, | |||
| StandardSyncInput syncInput); | |||
| StandardSyncInput syncInput, | |||
| final String taskQueue); | |||
There was a problem hiding this comment.
Talked about this on zoom, but to recap in writing:
Because this is changing the signature of an ActivityMethod, we should be careful around how this would affect any running workflows when we deploy the change.
I think that Temporal will basically replay the activity by passing in 'null' for any existing workflows that started before this was added, so we should make sure the code can handle 'null' as an input. Might be good to add a @Nullable annotation here with a comment explaining that this is to handle backwards compatibility with older Temporal workflows.
Once we're confident the older workflows are all finished, we can remove the @nullable annotation and cleanup if we'd like.
There was a problem hiding this comment.
@xiaohansong let me know when you've addressed the Temporal versioning / backwards compatibility concern and I can review again! Also let me know if you have any questions along the way!
|
@pmossman tested and it worked without failing! (Previously was failing due to Optional.of not accepting null value) |
...ence/job-persistence/src/test/java/io/airbyte/persistence/job/DefaultJobPersistenceTest.java
Outdated
Show resolved
Hide resolved
| @@ -116,7 +117,8 @@ public ReplicationActivityImpl(@Named("containerOrchestratorConfig") final Optio | |||
| public StandardSyncOutput replicate(final JobRunConfig jobRunConfig, | |||
| final IntegrationLauncherConfig sourceLauncherConfig, | |||
| final IntegrationLauncherConfig destinationLauncherConfig, | |||
| final StandardSyncInput syncInput) { | |||
| final StandardSyncInput syncInput, | |||
| @Nullable final String taskQueue) { | |||
There was a problem hiding this comment.
I think it'd be worth adding a comment explaining that this is only nullable so that workflow executions from before the change still work. Could also add a TODO comment that we can make this non-nullable in the future once there aren't any older workflow executions still running
| @@ -0,0 +1 @@ | |||
| null | |||
There was a problem hiding this comment.
looks like these got accidentally committed
* save queue name * make input nullable because we changed signature * PR Comments fix
What
#15898
save queue name into Attempts table so we can analysis and monitor data by data plane.