-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Destinations CDK: generation_id/sync_id plumbing #38358
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
version=0.35.14 | ||
version=0.36.0 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -140,8 +140,10 @@ abstract class JdbcSqlGeneratorIntegrationTest<DestinationState : MinimumDestina | |
includeCdcDeletedAt: Boolean, | ||
streamId: StreamId, | ||
suffix: String?, | ||
records: List<JsonNode> | ||
records: List<JsonNode>, | ||
generationId: Long, | ||
) { | ||
// TODO handle generation ID | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @gisripa I'm leaving the jdbc sqlgenerator integration test unimplemented for now. It's probably (?) pattern-matchable from https://github.com/airbytehq/airbyte/pull/38359/files#diff-39dbfe0f01e0b6a3efd7e11401344874227b2e30e04229391b38cbb9893913b2 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ack. Seems similar. for now its just no-op for old jdbc connectors right ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep. in theory this means they'll continue to build+test successfully, though... I haven't validated that in any way |
||
val columnNames = | ||
if (includeCdcDeletedAt) FINAL_TABLE_COLUMN_NAMES_CDC else FINAL_TABLE_COLUMN_NAMES | ||
insertRecords( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
|
||
package io.airbyte.integrations.base.destination.operation | ||
|
||
import io.airbyte.cdk.integrations.base.JavaBaseConstants.AIRBYTE_META_SYNC_ID_KEY | ||
import io.airbyte.cdk.integrations.destination.StreamSyncSummary | ||
import io.airbyte.cdk.integrations.destination.async.model.PartialAirbyteMessage | ||
import io.airbyte.cdk.integrations.destination.operation.SyncOperation | ||
|
@@ -15,6 +16,7 @@ import io.airbyte.integrations.base.destination.typing_deduping.StreamId | |
import io.airbyte.integrations.base.destination.typing_deduping.TyperDeduperUtil as tdutils | ||
import io.airbyte.integrations.base.destination.typing_deduping.migrators.Migration | ||
import io.airbyte.integrations.base.destination.typing_deduping.migrators.MinimumDestinationState | ||
import io.airbyte.protocol.models.v0.AirbyteRecordMessageMeta | ||
import io.airbyte.protocol.models.v0.StreamDescriptor | ||
import io.github.oshai.kotlinlogging.KotlinLogging | ||
import java.util.concurrent.CompletableFuture | ||
|
@@ -102,7 +104,22 @@ class DefaultSyncOperation<DestinationState : MinimumDestinationState>( | |
override fun flushStream(descriptor: StreamDescriptor, stream: Stream<PartialAirbyteMessage>) { | ||
val streamConfig = | ||
parsedCatalog.getStream(descriptor.namespace ?: defaultNamespace, descriptor.name) | ||
streamOpsMap[streamConfig.id]?.writeRecords(streamConfig, stream) | ||
streamOpsMap[streamConfig.id]?.writeRecords( | ||
streamConfig, | ||
stream.map { record -> | ||
if (record.record!!.meta == null) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we really want to modify the record object? That's a lot of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think modifying the object makes sense, most destinations are just going to though I have some opinions about
|
||
record.record!!.meta = AirbyteRecordMessageMeta() | ||
} | ||
record.also { | ||
it.record!! | ||
.meta!! | ||
.setAdditionalProperty( | ||
AIRBYTE_META_SYNC_ID_KEY, | ||
streamConfig.syncId, | ||
) | ||
} | ||
}, | ||
) | ||
} | ||
|
||
override fun finalizeStreams(streamSyncSummaries: Map<StreamDescriptor, StreamSyncSummary>) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't expecting this enum to become useful this early!