Repository navigation
test(Spanner): Live: Sqlserver migration integration tests for datatype - #4244
Conversation
af6c1ab to
3e286ff
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4244 +/- ##
=============================================
+ Coverage 36.74% 56.86% +20.11%
- Complexity 749 8072 +7323
=============================================
Files 250 1164 +914
Lines 17325 74057 +56732
Branches 1805 8715 +6910
=============================================
+ Hits 6366 42112 +35746
- Misses 10436 29140 +18704
- Partials 523 2805 +2282
🚀 New features to boost your workflow:
|
pratickchokhani
changed the title
b536a0c to
fcb4884
Compare
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request adds support for SQL Server migration integration tests for data types. It introduces a new Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for Microsoft SQL Server to the Datastream-to-Spanner integration tests and resource managers, including a new CloudSqlServerResourceManager, SqlServerSource configuration, and comprehensive data type migration tests (SQLServerDatastreamToSpannerDataTypesIT). Feedback on these changes includes resolving a potential failure when dropping databases due to connection pool pinning, wrapping test teardowns in try-catch blocks to prevent resource leaks, using a thread-safe set for test instances, and providing a default fallback port (1433) for SQL Server connections.
5478fa5 to
fd1ae69
Compare
…g connector implementation, dialect adapter, and configuration mappings
… Server integration tests and data types integration test.
fd1ae69 to
466be87
Compare
dhwanilpatel
left a comment
dhwanilpatel
left a comment
There was a problem hiding this comment.
Overall Looks good. Couple of questions.
9056e1d
into
GoogleCloudPlatform:main
- Rebase on main after GoogleCloudPlatform#4244 (static SQL Server container, shared datatype IT). - Align existing SQL Server ITs with the merged approach (enableCdcOnAllTables, shared SQLServerITContainer, resilient teardown); base class unchanged from main. - Add live-Datastream ports of the manifest scenarios: Events, DDL (custom transformation, size conversion, identity), Timezone, Session (rename/drop, synthetic PK), SeparateShadowTable PK-focused, SingleDF sharded (@ignore until migration_shard_id is populated for sqlserver sources). - Drop ITs not listed in manifest.yaml (RetryAllDLQ, sharded RetryDLQ, TableAndIndexLimits) and the onboarding report.
No description provided.