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

Destination snowflake: pull in cdk update for refreshes bugfix #42505

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

edgao
Copy link
Contributor

@edgao edgao commented Jul 24, 2024

What

How

Review guide

User Impact

Can this PR be safely reverted and rolled back?

  • YES πŸ’š
  • NO ❌

@edgao edgao requested a review from a team as a July 24, 2024 18:01
Copy link

vercel bot commented Jul 24, 2024

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Comments Updated (UTC)
airbyte-docs βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback Aug 19, 2024 6:13pm

@octavia-squidington-iii octavia-squidington-iii added the area/connectors Connector related issues label Jul 24, 2024
Copy link
Contributor Author

edgao commented Jul 24, 2024

@edgao edgao force-pushed the edgao/refreshes_recover_from_real_raw_table branch from d59f5d8 to 3d9cf10 Compare July 24, 2024 18:19
@edgao edgao force-pushed the edgao/snowflake_refreshes_bugfix branch from 77b72fe to b985ca0 Compare July 24, 2024 18:20
@edgao edgao force-pushed the edgao/refreshes_recover_from_real_raw_table branch from 3d9cf10 to 341ae0d Compare July 26, 2024 17:45
@edgao edgao force-pushed the edgao/refreshes_recover_from_real_raw_table branch from 341ae0d to 8a53e44 Compare July 26, 2024 20:29
@edgao edgao force-pushed the edgao/snowflake_refreshes_bugfix branch from b985ca0 to f5a8432 Compare July 26, 2024 20:29
@edgao edgao force-pushed the edgao/refreshes_recover_from_real_raw_table branch from 8a53e44 to 3922cf7 Compare July 26, 2024 22:44
@edgao edgao force-pushed the edgao/snowflake_refreshes_bugfix branch from f5a8432 to 4ae80f3 Compare July 26, 2024 22:45
@edgao edgao force-pushed the edgao/refreshes_recover_from_real_raw_table branch from 3922cf7 to b9bf10d Compare July 29, 2024 16:33
@edgao edgao force-pushed the edgao/snowflake_refreshes_bugfix branch from 4ae80f3 to 5ff87eb Compare July 29, 2024 16:34
@edgao edgao force-pushed the edgao/refreshes_recover_from_real_raw_table branch 2 times, most recently from 087b55b to 7fdef81 Compare August 2, 2024 19:59
@edgao edgao force-pushed the edgao/snowflake_refreshes_bugfix branch from 5ff87eb to 789e96e Compare August 2, 2024 20:00
@edgao edgao force-pushed the edgao/refreshes_recover_from_real_raw_table branch 2 times, most recently from ad24e27 to 1d5f768 Compare August 9, 2024 22:46
@edgao edgao force-pushed the edgao/refreshes_recover_from_real_raw_table branch from 1d5f768 to dd2e935 Compare August 12, 2024 17:18
@edgao edgao force-pushed the edgao/snowflake_refreshes_bugfix branch from 789e96e to c5f785b Compare August 12, 2024 17:19
@edgao edgao force-pushed the edgao/refreshes_recover_from_real_raw_table branch from dd2e935 to 707b94d Compare August 12, 2024 18:40
@edgao edgao force-pushed the edgao/snowflake_refreshes_bugfix branch from c5f785b to 1498c42 Compare August 12, 2024 18:41
@edgao edgao changed the base branch from edgao/refreshes_recover_from_real_raw_table to edgao/more_refresh_tests August 12, 2024 18:41
@edgao edgao force-pushed the edgao/more_refresh_tests branch from 19305f5 to 23a01c7 Compare August 14, 2024 18:19
@edgao edgao force-pushed the edgao/snowflake_refreshes_bugfix branch 5 times, most recently from 9154766 to a84bd37 Compare August 14, 2024 21:47
@edgao edgao force-pushed the edgao/more_refresh_tests branch from 23a01c7 to 5fb7304 Compare August 15, 2024 16:26
@edgao edgao force-pushed the edgao/snowflake_refreshes_bugfix branch 3 times, most recently from 0e80ea3 to 445f5cc Compare August 16, 2024 16:47
import org.junit.jupiter.api.Test

private val LOGGER = KotlinLogging.logger {}

abstract class AbstractSnowflakeTypingDedupingTest : BaseTypingDedupingTest() {
private var databaseName: String? = null
private lateinit var databaseName: String
Copy link
Contributor Author

Choose a reason for hiding this comment

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

switching to lateinit b/c otherwise the compiler complains about databaseName potentially changing values between its assignment on line 53 and its usage in cleanAirbyteInternalTable on 57 >.>

Copy link
Contributor

Choose a reason for hiding this comment

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

This spotbugs thing is really confusing when it is ok with lateinit and when it is not (I need to figure out how to logically reason about this)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I still have no idea why spotbugs yells at everyone else about lateinit, I've never had it happen to me >.>

@edgao edgao force-pushed the edgao/snowflake_refreshes_bugfix branch 2 times, most recently from 60aeb76 to ce6c207 Compare August 16, 2024 18:06
@edgao edgao force-pushed the edgao/more_refresh_tests branch from 5fb7304 to 95ac402 Compare August 16, 2024 20:10
@edgao edgao force-pushed the edgao/snowflake_refreshes_bugfix branch from ce6c207 to da85a99 Compare August 16, 2024 20:10
@octavia-squidington-iii octavia-squidington-iii added area/documentation Improvements or additions to documentation and removed CDK Connector Development Kit labels Aug 16, 2024
@@ -100,7 +100,7 @@ class SnowflakeStorageOperation(
generationIdNode =
results.first().get(JavaBaseConstants.COLUMN_NAME_AB_GENERATION_ID.uppercase())
}
return generationIdNode?.asLong()
return generationIdNode?.asLong() ?: 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there any implications at the caller if it is interpreted as 0 instead of null ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, this is the same fix as #42506 (comment)

(i.e. a record with null gen ID should be treated as gen 0, not as belonging to "current generation")

Base automatically changed from edgao/more_refresh_tests to edgao/refreshes_recover_from_real_raw_table August 19, 2024 16:27
@edgao edgao force-pushed the edgao/refreshes_recover_from_real_raw_table branch 2 times, most recently from 7270022 to 840e346 Compare August 19, 2024 17:15
Base automatically changed from edgao/refreshes_recover_from_real_raw_table to master August 19, 2024 18:07
@edgao edgao force-pushed the edgao/snowflake_refreshes_bugfix branch from da85a99 to 5cfdc8d Compare August 19, 2024 18:08
@edgao edgao merged commit 7ba3e2d into master Aug 20, 2024
34 checks passed
@edgao edgao deleted the edgao/snowflake_refreshes_bugfix branch August 20, 2024 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/destination/snowflake
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants