Skip to content

Formatting changes to templates#1667

Merged
liferoad merged 21 commits intoGoogleCloudPlatform:mainfrom
sharan-malyala:sharantej-readme
Dec 12, 2024
Merged

Formatting changes to templates#1667
liferoad merged 21 commits intoGoogleCloudPlatform:mainfrom
sharan-malyala:sharantej-readme

Conversation

@sharan-malyala
Copy link
Copy Markdown
Contributor

@sharan-malyala sharan-malyala commented Jun 18, 2024

Fixed formatting.

@sharan-malyala sharan-malyala marked this pull request as ready for review June 19, 2024 05:50
@sharan-malyala sharan-malyala requested a review from a team as a code owner June 19, 2024 05:50
@sharan-malyala sharan-malyala requested review from Deep1998 and manitgupta and removed request for a team June 19, 2024 05:50
@sharan-malyala sharan-malyala marked this pull request as draft June 21, 2024 04:38
@sharan-malyala sharan-malyala marked this pull request as ready for review June 21, 2024 04:39
@liferoad liferoad requested a review from rszper July 1, 2024 17:08
Copy link
Copy Markdown
Contributor

@rszper rszper left a comment

Choose a reason for hiding this comment

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

Thank you for making these updates. It's going to look much better.

sharan-malyala and others added 4 commits July 1, 2024 23:48
@sharan-malyala
Copy link
Copy Markdown
Contributor Author

@rszper Please confirm if these changes are good ? If yes, I will proceed to generate readme docs.

@sharan-malyala sharan-malyala requested a review from rszper July 12, 2024 04:23
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 45.96%. Comparing base (add3dd7) to head (fc8b3aa).
Report is 18 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1667      +/-   ##
============================================
+ Coverage     45.56%   45.96%   +0.39%     
- Complexity     3721     4124     +403     
============================================
  Files           847      851       +4     
  Lines         50169    50805     +636     
  Branches       5273     5343      +70     
============================================
+ Hits          22862    23351     +489     
- Misses        25632    25760     +128     
- Partials       1675     1694      +19     
Components Coverage Δ
spanner-templates 67.57% <ø> (+0.56%) ⬆️
spanner-import-export 65.47% <ø> (+1.29%) ⬆️
spanner-live-forward-migration 76.30% <ø> (+0.39%) ⬆️
spanner-live-reverse-replication 77.25% <ø> (+0.38%) ⬆️
spanner-bulk-migration 86.87% <ø> (+0.30%) ⬆️
Files with missing lines Coverage Δ
...loud/teleport/plugin/model/ImageSpecParameter.java 43.91% <100.00%> (ø)
...google/cloud/teleport/bigtable/AvroToBigtable.java 62.26% <ø> (ø)
...google/cloud/teleport/bigtable/BigtableToJson.java 61.95% <ø> (ø)
...gle/cloud/teleport/bigtable/BigtableToParquet.java 37.50% <ø> (ø)
.../teleport/bigtable/BigtableToVectorEmbeddings.java 78.00% <ø> (ø)
...e/cloud/teleport/bigtable/CassandraToBigtable.java 0.00% <ø> (ø)
...gle/cloud/teleport/bigtable/ParquetToBigtable.java 57.14% <ø> (ø)
.../google/cloud/teleport/spanner/ExportPipeline.java 0.00% <ø> (ø)
...gle/cloud/teleport/spanner/TextImportPipeline.java 0.00% <ø> (ø)
...e/cloud/teleport/templates/BigQueryToTFRecord.java 19.69% <ø> (ø)
... and 27 more

... and 38 files with indirect coverage changes

@sharan-malyala sharan-malyala requested a review from a team as a code owner September 19, 2024 06:44
@sharan-malyala sharan-malyala requested a review from a team as a code owner September 19, 2024 06:44
yair-harel
yair-harel previously approved these changes Sep 19, 2024
Copy link
Copy Markdown
Contributor

@bharadwaj-aditya bharadwaj-aditya left a comment

Choose a reason for hiding this comment

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

Comment for Spanner templates - Minor comment on the import pipeline. rest looks fine.

Copy link
Copy Markdown
Contributor

@ron-gal ron-gal left a comment

Choose a reason for hiding this comment

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

Minor format comments, otherwise LGTM for bigtable changes

Copy link
Copy Markdown
Contributor

@rszper rszper left a comment

Choose a reason for hiding this comment

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

It looks like there's a lot of repetition, so the edits that I suggested should be applied to the later files as well.

Polber
Polber previously approved these changes Sep 19, 2024
Copy link
Copy Markdown
Contributor

@Polber Polber left a comment

Choose a reason for hiding this comment

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

LGTM so long as other comments are addressed

Co-authored-by: Rebecca Szper <98840847+rszper@users.noreply.github.com>
@sharan-malyala sharan-malyala dismissed stale reviews from Polber and yair-harel via 0daaae1 October 31, 2024 08:19
@liferoad liferoad added Documentation improvement Making existing code better labels Dec 10, 2024
Copy link
Copy Markdown
Contributor

@liferoad liferoad left a comment

Choose a reason for hiding this comment

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

Tests are green.

@liferoad liferoad merged commit ff60a5f into GoogleCloudPlatform:main Dec 12, 2024
@sharan-malyala sharan-malyala deleted the sharantej-readme branch December 12, 2024 00:08
outputDirectory = "<outputDirectory>"
# sourceDbDialect = "MYSQL"
# jdbcDriverJars = "gs://your-bucket/driver_jar1.jar,gs://your-bucket/driver_jar2.jar"
# jdbcDriverJars = ""
Copy link
Copy Markdown
Contributor

@VardhanThigle VardhanThigle Dec 12, 2024

Choose a reason for hiding this comment

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

Could we add "<gsPathToDriverJars>" here?

# numPartitions = "0"
# spannerHost = "https://batch-spanner.googleapis.com"
# maxConnections = "-1"
# maxConnections = "0"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Generally -1 is preferred (which is also the default). -1 indicates no limits on connections. The user can configure a non-negaitve value in case they need to reduce the number of active connections per worker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Documentation improvement Making existing code better size/XXL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants