-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[Fix IT] Fix CassandraAllDataTypesIT failing test #3111
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
[Fix IT] Fix CassandraAllDataTypesIT failing test #3111
Conversation
Summary of ChangesHello @aasthabharill, 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 resolves a failing integration test by enhancing the robustness of the Cassandra driver configuration loading process. Previously, encountering an unknown Cassandra option would lead to an 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 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 counter productive. 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3111 +/- ##
============================================
+ Coverage 50.51% 50.67% +0.16%
- Complexity 5046 5053 +7
============================================
Files 973 974 +1
Lines 59744 59828 +84
Branches 6512 6527 +15
============================================
+ Hits 30177 30318 +141
+ Misses 27438 27367 -71
- Partials 2129 2143 +14
🚀 New features to boost your workflow:
|
.../java/com/google/cloud/teleport/v2/spanner/migrations/utils/CassandraDriverConfigLoader.java
Show resolved
Hide resolved
1852cb2 to
7775dc1
Compare
dfe98ea to
eec12a5
Compare
manitgupta
left a comment
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.
Let’s wait for all the Cassandra tests to pass before merging.
…m#3111) https://b.corp.google.com/issues/468909414 #### RCA - reference.conf file using to construct cassandra configuration comes from org/apache/cassandra which is at version 4.18.1 but all other related classes (for eg. TypedDriverOption) come from com/datastax/oss which is at version 4.17.0. This mismatch is causing newer added options/parameters to fail. #### Fix: com.datastax.oss version upgrade to 4.18.1 - this is actually invalid ever since datastax moved to apache so we now have to use the new library org.apache.cassandra with updated version 4.18.1
* Add IcebergToPostgres YAML template * Add filter parameter to Iceberg read configuration- Added filter (str) parameter to ReadFromIceberg transform in IcebergToPostgres.yaml- Updated IcebergToPostgresYaml.java interface with filter parameter between drop and keep- Adjusted order numbers for subsequent parameters- Added filter parameter to iceberg_options.yaml with example * Fix catalog name to hadoop_catalog in IcebergToPostgresYamlIT * Initialize Iceberg namespace before table creation * Fix file formatting * build(deps): bump org.junit.jupiter:junit-jupiter-api (#3131) Bumps [org.junit.jupiter:junit-jupiter-api](https://github.com/junit-team/junit-framework) from 6.0.0 to 6.0.1. - [Release notes](https://github.com/junit-team/junit-framework/releases) - [Commits](junit-team/junit-framework@r6.0.0...r6.0.1) --- updated-dependencies: - dependency-name: org.junit.jupiter:junit-jupiter-api dependency-version: 6.0.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * [SourceDbToSpanner] Reducing the dlqRetryMinutes and Correcting BulkAndLiveSpannerFT (#3132) * Correcting BulkAndLiveSpannerFT * Adding the worker machine type parameter * [Fix IT] Fix CassandraAllDataTypesIT failing test (#3111) https://b.corp.google.com/issues/468909414 #### RCA - reference.conf file using to construct cassandra configuration comes from org/apache/cassandra which is at version 4.18.1 but all other related classes (for eg. TypedDriverOption) come from com/datastax/oss which is at version 4.17.0. This mismatch is causing newer added options/parameters to fail. #### Fix: com.datastax.oss version upgrade to 4.18.1 - this is actually invalid ever since datastax moved to apache so we now have to use the new library org.apache.cassandra with updated version 4.18.1 * Add integration tests to verify table and index limits for bulk spanner migration from PostgreSQL (#3103) * Add check on max STRING length and also run for PG dialect Note, this also updates the length of the BYTES row that we migrate because the previous value did not actually exceed the limit (it ended up being 7MiB, less than the 10MiB max). This now writes a value that's actually larger than the limit and verifies that the row is NOT saved. * Add check on max row size for both dialects Currently ignored because it causes OOMs on low-provisioned VMs/DBs * Fix spotless * Add integration tests to check string and cell max size limits when migrating from Postgres * Add integration tests to check primary key and index max size limits when migrating from Postgres * Add integration tests to check row max size limits when migrating from Postgres * Support Float as primary key for Mysql source (#3016) * Support Float as primary key for Mysql source * add reference to mysql docs for the Float index * Add new Source IndexType float that map to java Float.class * Fix test for Indexes discovery (Float has its own index type now, not mapped a IndexType.OTHER) * Add TODOs for support Boundary.isSplittable on Approximate values (Float, Double) * Add integration tests * fix * - Add DecimalStepSize property to SourceColumnIndexInfo which is then passed to PartitionColumn. This is to allow the Boundary splitter to know what decimal precision is used by the source column. - Update Boundary.isSplittable to support FLOAT by doing delta based compare between 2 values. Either use a small default delta step size or the one provided the source db column. - Update MysqlDialectAdapter to pass the Numeric-scale schema value of Float column as DecimalStepSize to SourceColumnIndexInfo * syntax improvement * comment and clean diff * Add unit tests for BoundaryExtractorFactory Float.isSpittable custom behavior (with DecimalsStep paramater) * formatting * fix year index unit test * Add test for MysqlDialectAdapter, index NUMERIC_SCALE_COL * comment * Move default decimalStep from Boundary to MysqlDialectAdapter * fix PartitionColumn decimalStep preconditions * Add tests of Boundary.isMergable with float values (check that it works with approximate delta based comparisons) * Formating with mvn:spotless * build bigdecimal step default from String instead of float (to avoid approximation) * Fix tests * Address PR feedback --------- Co-authored-by: Nicolas Emond <[email protected]> * Update README with filter parameter documentation --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Ulada Butsenka <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: darshan-sj <[email protected]> Co-authored-by: aasthabharill <[email protected]> Co-authored-by: Nicolas Emond <[email protected]> Co-authored-by: Nicolas Riche <[email protected]>
https://b.corp.google.com/issues/468909414
RCA
Fix:
com.datastax.oss version upgrade to 4.18.1 - this is actually invalid ever since datastax moved to apache so we now have to use the new library org.apache.cassandra with updated version 4.18.1
Debugging:
When i checked the reference.conf file in the java driver core we build (/.m2/repository/org/apache/cassandra/java-driver-core/4.18.1/java-driver-core-4.18.1.jar!/reference.conf) it had the line
preferred-remote-dcs = [""].This reference.conf is automatically merged with our specified .conf file (Documentation: "Since application.conf inherits from reference.conf, you only need to redeclare what you override")
We strictly check if all the options specified in the final configuration are "valid" configurations by using the class TypedDriverOption (path in jar: /Users/aasthabharill/.m2/repository/com/datastax/oss/java-driver-core/4.17.0/java-driver-core-4.17.0.jar!/com/datastax/oss/driver/api/core/config/TypedDriverOption.class)
Problem:
preferred-remote-dcs = [""]field.preferred-remote-dcsin its reference.conf