Skip to content
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

CDK module changes for destination #36588

Merged
merged 1 commit into from
Mar 29, 2024
Merged

Conversation

gisripa
Copy link
Contributor

@gisripa gisripa commented Mar 28, 2024

TL;DR

Updated JDBC and S3 destination configurations.

Copy link

vercel bot commented Mar 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
airbyte-docs ⬜️ Ignored (Inspect) Visit Preview Mar 28, 2024 11:47pm

@octavia-squidington-iii octavia-squidington-iii added the CDK Connector Development Kit label Mar 28, 2024
Copy link
Contributor Author

gisripa commented Mar 28, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @gisripa and the rest of your teammates on Graphite Graphite

@gisripa gisripa marked this pull request as ready for review March 28, 2024 01:31
@gisripa gisripa requested review from a team as code owners March 28, 2024 01:31
@gisripa gisripa force-pushed the gireesh/cdkk-destination-fixes branch 3 times, most recently from c44486f to 5610dfe Compare March 28, 2024 01:55
fun uploadManifest(bucketName: String, manifestFilePath: String, manifestContents: String) {
s3Client.putObject(s3Config.bucketName, manifestFilePath, manifestContents)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method got nuked out of orbit.. diff is just resurrection.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops

@gisripa gisripa force-pushed the gireesh/cdkk-destination-fixes branch 2 times, most recently from 183757d to d396108 Compare March 28, 2024 03:04
@@ -44,9 +44,10 @@ object Jsons {
private val YAML_OBJECT_MAPPER: ObjectMapper = MoreMappers.initYamlMapper(YAMLFactory())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Re: line 29]

This whole class should be just companion object.

See this comment inline on Graphite.

@gisripa gisripa force-pushed the gireesh/cdkk-destination-fixes branch 5 times, most recently from 224bcb8 to 7466653 Compare March 28, 2024 17:45
Copy link
Contributor

@jdpgrailsdev jdpgrailsdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢

@gisripa gisripa force-pushed the gireesh/cdkk-destination-fixes branch from 7466653 to 08d1dcd Compare March 28, 2024 18:25
@gisripa gisripa force-pushed the gireesh/cdkk-destination-fixes branch 2 times, most recently from 9370fc8 to cd4a7c2 Compare March 28, 2024 22:41
@@ -14,18 +14,8 @@ package io.airbyte.integrations.base.destination.typing_deduping
* Useful if a destination warehouse handles columns ignoring case, but preserves case in the table
* schema.
*/
class ColumnId(name: String, originalName: String, canonicalName: String) {
data class ColumnId(val name: String, val originalName: String, val canonicalName: String) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you

) {
val rawTableExists: Boolean
val hasUnprocessedRecords: Boolean
data class InitialRawTableStatus(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you

@@ -58,7 +58,7 @@ abstract class BaseSqlGeneratorIntegrationTest<DestinationState : MinimumDestina
protected var cdcIncrementalAppendStream: StreamConfig? = null

protected var generator: SqlGenerator? = null
protected abstract var destinationHandler: DestinationHandler<DestinationState>
protected abstract val destinationHandler: DestinationHandler<DestinationState>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure why it was defined as a var... Much better as a val. How did you find it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

making var generated abstract setDestinationHandler method which was never intended to be, just getDestinationHandler

@gisripa gisripa force-pushed the gireesh/cdkk-destination-fixes branch from cd4a7c2 to f302124 Compare March 28, 2024 23:46
@gisripa gisripa merged commit 219c194 into master Mar 29, 2024
29 checks passed
@gisripa gisripa deleted the gireesh/cdkk-destination-fixes branch March 29, 2024 17:08
nurikk pushed a commit to nurikk/airbyte that referenced this pull request Apr 4, 2024
### TL;DR
Updated JDBC and S3 destination to compile destination-redshift
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CDK Connector Development Kit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants