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.

Sign up for GitHub

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

Jump to bottom

Modified parameters for BigtableToParquet #1490

Merged
merged 24 commits into from May 10, 2024
Merged

Modified parameters for BigtableToParquet #1490

merged 24 commits into from May 10, 2024

Conversation

Supriya-Koppa
Copy link
Contributor

@Supriya-Koppa Supriya-Koppa commented Apr 30, 2024 •
edited

VeronicaWasson
VeronicaWasson approved these changes Apr 30, 2024
View reviewed changes
Copy link

@VeronicaWasson VeronicaWasson left a comment

Choose a reason for hiding this comment

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

I suggested some minor edits, otherwise LGTM

@pull-request-size pull-request-size bot added size/M and removed size/S labels May 1, 2024
@Supriya-Koppa Supriya-Koppa marked this pull request as draft May 1, 2024 09:44
@pull-request-size pull-request-size bot added size/L and removed size/M labels May 1, 2024
@Supriya-Koppa Supriya-Koppa marked this pull request as ready for review May 6, 2024 14:10
damccorm
damccorm reviewed May 6, 2024
View reviewed changes
Copy link
Contributor

@damccorm damccorm left a comment

Choose a reason for hiding this comment

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

Please run mvn spotless:apply, once that is passing we can go ahead and merge

rszper
rszper reviewed May 6, 2024
View reviewed changes
v1/src/main/java/com/google/cloud/teleport/bigtable/BigtableToParquet.java Outdated Show resolved Hide resolved
v1/src/main/java/com/google/cloud/teleport/bigtable/BigtableToParquet.java Outdated Show resolved Hide resolved
v1/src/main/java/com/google/cloud/teleport/bigtable/BigtableToParquet.java Outdated Show resolved Hide resolved
v1/src/main/java/com/google/cloud/teleport/templates/SpannerVectorEmbeddingExport.java Outdated Show resolved Hide resolved
v1/src/main/java/com/google/cloud/teleport/templates/SpannerVectorEmbeddingExport.java Outdated Show resolved Hide resolved
...ud-to-googlecloud/src/main/java/com/google/cloud/teleport/v2/templates/TextIOToBigQuery.java Outdated Show resolved Hide resolved
...oud/src/main/java/com/google/cloud/teleport/v2/mongodb/options/MongoDbToBigQueryOptions.java Outdated Show resolved Hide resolved
...oud/src/main/java/com/google/cloud/teleport/v2/mongodb/options/MongoDbToBigQueryOptions.java Outdated Show resolved Hide resolved
...oud/src/main/java/com/google/cloud/teleport/v2/mongodb/options/MongoDbToBigQueryOptions.java Outdated Show resolved Hide resolved
...oud/src/main/java/com/google/cloud/teleport/v2/mongodb/options/MongoDbToBigQueryOptions.java Outdated Show resolved Hide resolved
Supriya-Koppa and others added 2 commits May 7, 2024 10:46
@Supriya-Koppa @rszper
Apply suggestions from code review
a1d02b5 
Co-authored-by: Rebecca Szper <98840847+rszper@users.noreply.github.com>
@Supriya-Koppa
maven spoteless
acf5569
rszper
rszper approved these changes May 7, 2024
View reviewed changes
Copy link
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.

Just a few remaining comments; otherwise, LGTM.

v1/src/main/java/com/google/cloud/teleport/templates/common/DatadogConverters.java Outdated Show resolved Hide resolved
v1/src/main/java/com/google/cloud/teleport/templates/common/DatastoreConverters.java Outdated Show resolved Hide resolved
v1/src/main/java/com/google/cloud/teleport/templates/common/SplunkConverters.java Outdated Show resolved Hide resolved
v1/src/main/java/com/google/cloud/teleport/templates/common/SplunkConverters.java Outdated Show resolved Hide resolved
v1/src/main/java/com/google/cloud/teleport/templates/common/SplunkConverters.java Outdated Show resolved Hide resolved
gauravjain6633 and others added 4 commits May 8, 2024 07:00
@gauravjain6633
Removing optional from helptext
b48de7a
@Supriya-Koppa @rszper
Update v1/src/main/java/com/google/cloud/teleport/templates/common/Da…
ffd8d07 
…tadogConverters.java

Co-authored-by: Rebecca Szper <98840847+rszper@users.noreply.github.com>
@Supriya-Koppa @rszper
Apply suggestions from code review
a0744c0 
Co-authored-by: Rebecca Szper <98840847+rszper@users.noreply.github.com>
@Supriya-Koppa @rszper
Update v1/src/main/java/com/google/cloud/teleport/templates/common/Da…
6422bf7 
…tastoreConverters.java

Co-authored-by: Rebecca Szper <98840847+rszper@users.noreply.github.com>
@codecov Codecov
Copy link

codecov bot commented May 8, 2024 •
edited

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 40.26%. Comparing base (c7d8f3f) to head (f56eac6).
Report is 42 commits behind head on main.

Additional details and impacted files 
@@             Coverage Diff              @@
##               main    #1490      +/-   ##
============================================
+ Coverage     40.08%   40.26%   +0.17%     
- Complexity     2803     2813      +10     
============================================
  Files           732      740       +8     
  Lines         42615    42899     +284     
  Branches       4581     4604      +23     
============================================
+ Hits          17081    17272     +191     
- Misses        24052    24134      +82     
- Partials       1482     1493      +11
Components Coverage Δ
spanner-templates 56.80% <ø> (-0.28%) ⬇️
spanner-import-export 65.52% <ø> (-0.07%) ⬇️
spanner-live-forward-migration 63.17% <ø> (+0.55%) ⬆️
spanner-live-reverse-replication 42.95% <ø> (+0.39%) ⬆️
spanner-bulk-migration 69.97% <ø> (-2.17%) ⬇️
Files Coverage Δ
...gle/cloud/teleport/bigtable/BigtableToParquet.java 41.86% <ø> (ø)
...leport/templates/SpannerVectorEmbeddingExport.java 0.00% <ø> (ø)
...d/teleport/templates/common/DatadogConverters.java 81.25% <ø> (ø)
...teleport/templates/common/DatastoreConverters.java 62.67% <ø> (ø)
...rt/templates/common/JavascriptTextTransformer.java 66.22% <ø> (ø)
...ud/teleport/templates/common/PubsubConverters.java 0.00% <ø> (ø)
...ud/teleport/templates/common/SplunkConverters.java 80.55% <ø> (ø)
...cloud/teleport/v2/templates/BigQueryToParquet.java 1.63% <ø> (ø)
...dc/applier/CdcToBigQueryChangeApplierPipeline.java 0.00% <ø> (ø)
...ud/teleport/v2/templates/DataStreamToBigQuery.java 0.00% <ø> (ø)
... and 2 more

... and 31 files with indirect coverage changes

@Supriya-Koppa
Copy link
Contributor Author

@damccorm - 1 workflow awaiting approval from a maintainer - could you please check? PR is ready from my end. Thanks!

@damccorm
Copy link
Contributor

damccorm commented May 9, 2024

Please run mvn spotless:apply

@damccorm damccorm added the Google LGTM Approval of a pull request to be merged into the repository label May 9, 2024
@copybara-service copybara-service bot merged commit 1b10b2e into GoogleCloudPlatform:main May 10, 2024
14 checks passed
@Abacn Abacn mentioned this pull request May 10, 2024
Closed
@Polber Polber mentioned this pull request May 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@damccorm damccorm damccorm approved these changes

@gauravjain6633 gauravjain6633 gauravjain6633 left review comments

@VeronicaWasson VeronicaWasson VeronicaWasson approved these changes

@rszper rszper rszper approved these changes

Assignees
No one assigned
Labels
Google LGTM Approval of a pull request to be merged into the repository size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Supriya-Koppa @damccorm @VeronicaWasson @rszper @gauravjain6633 @sharan-malyala