-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Bulk Load CDK: Simply Interface & Add Check #45369
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 |
---|---|---|
@@ -0,0 +1,28 @@ | ||
/* | ||
* Copyright (c) 2024 Airbyte, Inc., all rights reserved. | ||
*/ | ||
|
||
package io.airbyte.cdk.check | ||
|
||
import io.airbyte.cdk.Operation | ||
import io.airbyte.cdk.output.ExceptionHandler | ||
import io.micronaut.context.annotation.Requires | ||
import jakarta.inject.Singleton | ||
|
||
@Singleton | ||
@Requires(property = Operation.PROPERTY, value = "check") | ||
@Requires(env = ["destination"]) | ||
class CheckOperation( | ||
private val destination: DestinationCheck, | ||
private val exceptionHandler: ExceptionHandler, | ||
) : Operation { | ||
override fun execute() { | ||
try { | ||
destination.check() | ||
} catch (e: Exception) { | ||
exceptionHandler.handle(e) | ||
} finally { | ||
destination.cleanup() | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
/* | ||
* Copyright (c) 2024 Airbyte, Inc., all rights reserved. | ||
*/ | ||
|
||
package io.airbyte.cdk.check | ||
|
||
interface DestinationCheck { | ||
fun check() | ||
fun cleanup() {} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,26 +9,30 @@ import io.micronaut.context.annotation.Secondary | |
import jakarta.inject.Singleton | ||
|
||
/** | ||
* Implementor interface. Extended this only if you need to perform initialization and teardown | ||
* *across all streams*, or if your per-stream operations need shared global state. | ||
* | ||
* If initialization can be done on a per-stream basis, implement @[StreamLoaderFactory] instead. | ||
* Implementor interface. Every Destination must extend this and at least provide an implementation | ||
* of [getStreamLoader]. | ||
*/ | ||
interface Destination { | ||
interface DestinationWrite { | ||
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. not in love with the name. 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. NVM, There s a WriteOperation down below. Begs the question why we need both though 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. See previous comment. |
||
// Called once before anything else | ||
suspend fun setup() {} | ||
|
||
// Return a StreamLoader for the given stream | ||
fun getStreamLoader(stream: DestinationStream): StreamLoader | ||
|
||
// Called once at the end of the job | ||
// Called once at the end of the job, unconditionally. | ||
suspend fun teardown(succeeded: Boolean = true) {} | ||
} | ||
|
||
@Singleton | ||
@Secondary | ||
class DefaultDestination(private val streamLoaderFactory: StreamLoaderFactory) : Destination { | ||
class DefaultDestinationWrite : DestinationWrite { | ||
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 need a default that crashes? Might be worth explaining why 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. Can't hurt to tell the implementor what's missing 🤷 |
||
init { | ||
throw NotImplementedError( | ||
"DestinationWrite not implemented. Please create a custom @Singleton implementation." | ||
) | ||
} | ||
|
||
override fun getStreamLoader(stream: DestinationStream): StreamLoader { | ||
return streamLoaderFactory.make(stream) | ||
throw NotImplementedError() | ||
} | ||
} |
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.
any reason why we wouldn't want an abstract class here, with the check() as an abstract function
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.
The
@Requires
aren't inherited. It would force each implementor to specify the conditions under which their implementation is run.