DataflowTemplates icon indicating copy to clipboard operation
DataflowTemplates copied to clipboard

Organize it module and use system properties for it/jdbc

Open Polber opened this issue 1 year ago • 2 comments

The purpose of this PR is to reorganize the IT module and remove unused code.

A lot of the classes in the IT module have become disjoint from the Beam source as mentioned in https://github.com/apache/beam/issues/30206.

Many of the classes in our port of the it/ module are 1:1 duplicates. Those duplicates have been removed, and only files with repo-specific changes have been left behind.

Alongside removing unused code, this PR also reorganizes templates-specific it classes into appropriate locations. For example, the JDBCBaseIT class was under it/google-cloud-platform, when a more appropriate location is a separate it/jdbc folder. Same goes for moving load test classes under the new lt/ module rather than under it/.

Lastly, the JDBC drivers used for testing have been consolidated under the it/jdbc module rather than defining as test artifacts in all the individual template modules that use them. The JDBCBaseIT also will now use the versions defined in the it/jdbc/pom.xml file instead of hard-coding the values in the class. This will make maintaining these dependencies a cleaner experience.


If the goal, according to https://github.com/apache/beam/issues/30206, is to in fact move IT classes back to the DataflowTemplates repo, a next step would be to either

  1. Merge the classes left behind by this change into Beam, to get a single source-of-truth, then merge the entire module back into Templates
  2. Go ahead and move all the IT classes into DataflowTemplates from Beam and handle merge conflicts manually.

Polber avatar Oct 09 '24 22:10 Polber

Codecov Report

:x: Patch coverage is 0% with 29 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 45.18%. Comparing base (8c035b9) to head (02cb09e). :warning: Report is 602 commits behind head on main.

Files with missing lines Patch % Lines
.../com/google/cloud/teleport/it/jdbc/JDBCBaseIT.java 0.00% 16 Missing :warning:
...main/java/org/apache/beam/it/gcp/LoadTestBase.java 0.00% 13 Missing :warning:

:x: Your patch status has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1923      +/-   ##
============================================
+ Coverage     44.80%   45.18%   +0.38%     
  Complexity     3563     3563              
============================================
  Files           832      801      -31     
  Lines         49479    48011    -1468     
  Branches       5196     5097      -99     
============================================
- Hits          22169    21695     -474     
+ Misses        25659    24692     -967     
+ Partials       1651     1624      -27     
Components Coverage Δ
spanner-templates 65.55% <ø> (-0.01%) :arrow_down:
spanner-import-export 63.80% <ø> (-0.03%) :arrow_down:
spanner-live-forward-migration 74.86% <ø> (ø)
spanner-live-reverse-replication 75.55% <ø> (ø)
spanner-bulk-migration 84.20% <ø> (ø)
Files with missing lines Coverage Δ
...va/com/google/cloud/teleport/TemplateTestBase.java 0.00% <ø> (ø)
...oud/teleport/it/jdbc/conditions/JDBCRowsCheck.java 0.00% <ø> (ø)
...main/java/org/apache/beam/it/gcp/LoadTestBase.java 0.00% <0.00%> (ø)
.../com/google/cloud/teleport/it/jdbc/JDBCBaseIT.java 0.00% <0.00%> (ø)

... and 4 files with indirect coverage changes

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Oct 10 '24 18:10 codecov[bot]

This pull request has been marked as stale due to 180 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time. Thank you for your contributions.

github-actions[bot] avatar Apr 14 '25 02:04 github-actions[bot]

This pull request has been marked as stale due to 180 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time. Thank you for your contributions.

github-actions[bot] avatar Oct 13 '25 02:10 github-actions[bot]