Skip to content

Conversation

@arawind
Copy link
Contributor

@arawind arawind commented Mar 19, 2024

Import / export tests in cloud-devel

Avro:

GoogleSQL:

PG:

CSV:

GoogleSQL:

PG:

@arawind arawind requested a review from a team as a code owner March 19, 2024 13:53
@arawind arawind requested review from darshan-sj and manitgupta and removed request for a team March 19, 2024 13:53
@manitgupta
Copy link
Member

This is an XL sized PR...is it possible to break this up?

Maybe Avro template changes in one and text template changes in another?

@manitgupta
Copy link
Member

I see some failures in TextImportIT, can you please check?
I will also review the PR in the meantime

@arawind
Copy link
Contributor Author

arawind commented Mar 21, 2024

Thanks!

I see some failures in TextImportIT, can you please check? I will also review the PR in the meantime

This failure is expected today as the type isn't released yet in Spanner. We're expecting it to land soon (like soon soon); I can let you know the exact details privately.

Copy link
Member

@manitgupta manitgupta left a comment

Choose a reason for hiding this comment

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

I don't see any changes ImportPipeline.java?
That is the Avro record to Spanner pipeline, is no change required in that?
Similarly don't see changes in the export templates as well - ExportPipeline.java

@arawind
Copy link
Contributor Author

arawind commented Mar 21, 2024

I don't see any changes ImportPipeline.java? That is the Avro record to Spanner pipeline, is no change required in that? Similarly don't see changes in the export templates as well - ExportPipeline.java

These files do not have any type related details -- they seem generic runners of the pipeline -- should there be any changes there?

Copy link
Contributor

@darshan-sj darshan-sj left a comment

Choose a reason for hiding this comment

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

Can you run an Export/Import and document it in the PR description?

@manitgupta
Copy link
Member

manitgupta commented Mar 21, 2024

I don't see any changes ImportPipeline.java? That is the Avro record to Spanner pipeline, is no change required in that? Similarly don't see changes in the export templates as well - ExportPipeline.java

These files do not have any type related details -- they seem generic runners of the pipeline -- should there be any changes there?

@darshan-sj

Logically speaking there should be? Your other changes point to converting Avro records and data types, so why won't this change? At the minimum, the IT should change to account for this type right? What am I missing - @darshan-sj ?

@arawind
Copy link
Contributor Author

arawind commented Mar 22, 2024

Can you run an Export/Import and document it in the PR description?

Done!

Logically speaking there should be? Your other changes point to converting Avro records and data types, so why won't this change? At the minimum, the IT should change to account for this type right? What am I missing - @darshan-sj ?

The Avro conversion changes are in AvroRecordConverter and AvroSchemaToDdlConverter. The actual pipelines don't need any change.

I've added a new integration test in ExportPipelineIT. Will work on adding ImportPipelineIT too in a bit

darshan-sj
darshan-sj previously approved these changes Mar 26, 2024
@codecov
Copy link

codecov bot commented Mar 27, 2024

Codecov Report

Merging #1376 (22c7c33) into main (980e7ab) will increase coverage by 0.06%.
The diff coverage is 78.08%.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1376      +/-   ##
============================================
+ Coverage     38.93%   38.99%   +0.06%     
- Complexity     2760     2789      +29     
============================================
  Files           743      743              
  Lines         41948    42020      +72     
  Branches       4516     4529      +13     
============================================
+ Hits          16332    16386      +54     
- Misses        24127    24137      +10     
- Partials       1489     1497       +8     
Components Coverage Δ
spanner-templates 52.39% <90.16%> (+0.22%) ⬆️
spanner-import-export 65.51% <90.16%> (+0.28%) ⬆️
spanner-live-forward-migration 56.23% <ø> (ø)
spanner-live-reverse-replication 37.53% <ø> (ø)
spanner-bulk-migration 58.12% <ø> (ø)
Files Coverage Δ
...le/cloud/teleport/spanner/AvroRecordConverter.java 99.69% <100.00%> (+0.02%) ⬆️
...oud/teleport/spanner/AvroSchemaToDdlConverter.java 80.38% <100.00%> (+0.38%) ⬆️
...oud/teleport/spanner/DdlToAvroSchemaConverter.java 95.05% <100.00%> (+0.02%) ⬆️
...gle/cloud/teleport/spanner/TextImportPipeline.java 0.00% <ø> (ø)
...oogle/cloud/teleport/spanner/common/SizedType.java 90.68% <100.00%> (+0.36%) ⬆️
.../com/google/cloud/teleport/spanner/ddl/Column.java 82.27% <100.00%> (+0.46%) ⬆️
...d/teleport/templates/common/SpannerConverters.java 75.91% <100.00%> (+0.32%) ⬆️
...cloud/teleport/spanner/SpannerRecordConverter.java 78.90% <75.00%> (-0.07%) ⬇️
...com/google/cloud/teleport/spanner/common/Type.java 89.17% <90.00%> (+0.04%) ⬆️
...le/cloud/teleport/spanner/CSVRecordToMutation.java 81.90% <0.00%> (-2.57%) ⬇️
... and 4 more

... and 1 file with indirect coverage changes

@darshan-sj darshan-sj added the Google LGTM Approval of a pull request to be merged into the repository label Mar 28, 2024
@arawind arawind closed this Apr 1, 2024
@arawind arawind reopened this Apr 1, 2024
@arawind
Copy link
Contributor Author

arawind commented Apr 1, 2024

Squashed my commits into 5764ef8 and force pushed to resolve the merge conflicts in copybara

@arawind arawind reopened this Apr 4, 2024
@arawind arawind force-pushed the float32 branch 2 times, most recently from dd41519 to 357481b Compare April 5, 2024 14:17
Squashed commit of the following:

commit 1e21615
Merge: 5764ef8 27e7486
Author: Aravind Pedapudi <aravindp1510@gmail.com>
Date:   Thu Apr 4 21:35:22 2024 +0530

    Merge branch 'main' into float32

commit 5764ef8
Author: Aravind <aravindp1510@gmail.com>
Date:   Mon Apr 1 17:40:34 2024 +0530

    Support FLOAT32 type in v1 templates of Spanner.

    Squashed commit of the following:

    commit a678a75
    Merge: f9fe874 c9f1c66
    Author: Aravind Pedapudi <aravindp1510@gmail.com>
    Date:   Mon Apr 1 16:08:20 2024 +0530

        Merge branch 'main' into float32

    commit f9fe874
    Merge: e28335c 7449ae6
    Author: Aravind Pedapudi <aravindp1510@gmail.com>
    Date:   Thu Mar 28 10:50:24 2024 +0530

        Merge branch 'main' into float32

    commit e28335c
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Wed Mar 27 16:33:56 2024 +0530

        Fix integration test failures for TextImportPipelineIT

    commit 8480654
    Merge: c6364b7 afd1dc2
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Wed Mar 27 16:25:57 2024 +0530

        Merge branch 'float32' of github.com:arawind/DataflowTemplates into float32

    commit c6364b7
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Wed Mar 27 16:22:27 2024 +0530

        Add avro integration tests for FLOAT32 type in Spanner.

        This fixes the ExportPipelineIT and adds support for the
        ImportPipelineIT.

    commit afd1dc2
    Merge: eca6acf 4536653
    Author: Aravind Pedapudi <aravindp1510@gmail.com>
    Date:   Wed Mar 27 11:37:24 2024 +0530

        Merge branch 'main' into float32

    commit eca6acf
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Fri Mar 22 17:12:35 2024 +0530

        Address review comments

        Adds new tests in ExportPipelineIT and also removes the INT32 -> FLOAT32 parsing as we don't see any use for it.

    commit c1cd281
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Thu Mar 21 18:15:05 2024 +0530

        Fix Beam to handle float32 arrays and pg schema.

        This was missed in the earlier commits

    commit 5e936f4
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Thu Mar 21 13:32:01 2024 +0530

        Address review comments

    commit f4ab908
    Merge: a6aba30 1df7e0a
    Author: Aravind Pedapudi <aravindp1510@gmail.com>
    Date:   Thu Mar 21 12:56:09 2024 +0530

        Merge branch 'GoogleCloudPlatform:main' into float32

    commit a6aba30
    Merge: 66c488c f44f7ce
    Author: Aravind Pedapudi <aravindp1510@gmail.com>
    Date:   Wed Mar 20 16:21:11 2024 +0530

        Merge branch 'main' into float32

    commit 66c488c
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Wed Mar 20 15:49:31 2024 +0530

        Fix gax dependency failures in spanner integration tests.

        We were seeing this error in the spanner integration tests: `class com.google.cloud.spanner.v1.stub.SpannerStubSettings overrides final method com.google.api.gax.rpc.StubSettings.getEndpoint()Ljava/lang/String;`, which were triggered by the google-cloud-spanner BOM update to 6.61.
        We're fixing these errors by setting the gax version to the one used by Cloud Spanner client libs. This should be removed once Beam is updated to the latest version of gax (which may happen when Beam's dependency on Spanner is updated).

    commit 90c1fbc
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Tue Mar 19 19:17:03 2024 +0530

        Support FLOAT32 type in v1 spanner templates

    commit 51f75f0
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Tue Mar 19 19:10:06 2024 +0530

        Modify the local copy of Beam to support FLOAT32 type.

    commit f85fd00
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Tue Mar 19 18:53:17 2024 +0530

        Update the version of google-cloud-spanner-bom to 6.61.0
@Polber Polber merged commit b51ff05 into GoogleCloudPlatform:main Apr 9, 2024
damccorm pushed a commit that referenced this pull request Apr 11, 2024
Please approve this CL. It will be submitted automatically, and its GitHub pull request will be marked as merged.

Imported from GitHub PR #1376

## Import / export tests in cloud-devel

### Avro:

GoogleSQL:
* [Export](https://console.cloud.google.com/dataflow/jobs/us-central1/2024-03-21_05_01_22-9618753751624897563;graphView=0?project=span-cloud-testing&pageState=(%22dfTime%22:(%22l%22:%22dfJobMaxTime%22)))
* [Import](https://console.cloud.google.com/dataflow/jobs/us-central1/2024-03-21_05_38_00-8680174928377892705;graphView=0?project=span-cloud-testing&pageState=(%22dfTime%22:(%22l%22:%22dfJobMaxTime%22)))

PG:
* [Export](https://console.cloud.google.com/dataflow/jobs/us-central1/2024-03-21_05_47_07-2771163877050158940;graphView=0?project=span-cloud-testing&pageState=(%22dfTime%22:(%22l%22:%22dfJobMaxTime%22)))
* [Import](https://console.cloud.google.com/dataflow/jobs/us-central1/2024-03-21_05_58_11-1643174307354839989;graphView=0?project=span-cloud-testing&pageState=(%22dfTime%22:(%22l%22:%22dfJobMaxTime%22)))

### CSV:

GoogleSQL:
* [Export](https://console.cloud.google.com/dataflow/jobs/us-central1/2024-03-22_00_17_56-5509502875882896332?e=13802955&mods=monitoring_api_prod&project=span-cloud-testing)
* [Import](https://console.cloud.google.com/dataflow/jobs/us-central1/2024-03-22_02_50_39-14326646282971458157?e=13802955&mods=monitoring_api_prod&project=span-cloud-testing)

PG:
* [Export](https://console.cloud.google.com/dataflow/jobs/us-central1/2024-03-22_03_17_13-5127588554893435640;graphView=0?project=span-cloud-testing&e=13802955&mods=monitoring_api_prod&pageState=(%22dfTime%22:(%22l%22:%22dfJobMaxTime%22)))
* [Import](https://console.cloud.google.com/dataflow/jobs/us-central1/2024-03-22_03_09_55-9477941764979866789?e=13802955&mods=monitoring_api_prod&project=span-cloud-testing)

Copybara import of the project:

  - 357481bb0fdfba9c91fdde1c24ae4bbcaf5d2bf4 Support FLOAT32 type in v1 templates of Spanner. by Aravind <aravindp1510@gmail.com>

COPYBARA_INTEGRATE_REVIEW=#1376 from arawind:float32 357481bb0fdfba9c91fdde1c24ae4bbcaf5d2bf4
PiperOrigin-RevId: 622232578
@Abacn
Copy link
Contributor

Abacn commented Apr 11, 2024

Please sync this update to Beam repo. LocalSpannerIO will be removed (#1362) and this change will get lost

@arawind
Copy link
Contributor Author

arawind commented Apr 12, 2024

@Abacn thanks for the ping. We're trying to get this change merged into 2.56 in apache/beam#30893

@arawind arawind deleted the float32 branch April 12, 2024 07:54
liferoad pushed a commit to aksharauke/DataflowTemplates that referenced this pull request Apr 18, 2024
Get consistent java-pr signal on main

Default to THROUGHPUT_BASED autoscaling for classic templates (GoogleCloudPlatform#1403)

* single commit off of master

* No-op comment change to kick copybara

* Revert "No-op comment change to kick copybara"

This reverts commit 928dc6b.

* Different no-op to kick copybara and avoid conflicts

* One more try to get copybara to merge

Support FLOAT32 type in v1 templates of Spanner. (GoogleCloudPlatform#1376)

Squashed commit of the following:

commit 1e21615
Merge: 5764ef8 27e7486
Author: Aravind Pedapudi <aravindp1510@gmail.com>
Date:   Thu Apr 4 21:35:22 2024 +0530

    Merge branch 'main' into float32

commit 5764ef8
Author: Aravind <aravindp1510@gmail.com>
Date:   Mon Apr 1 17:40:34 2024 +0530

    Support FLOAT32 type in v1 templates of Spanner.

    Squashed commit of the following:

    commit a678a75
    Merge: f9fe874 c9f1c66
    Author: Aravind Pedapudi <aravindp1510@gmail.com>
    Date:   Mon Apr 1 16:08:20 2024 +0530

        Merge branch 'main' into float32

    commit f9fe874
    Merge: e28335c 7449ae6
    Author: Aravind Pedapudi <aravindp1510@gmail.com>
    Date:   Thu Mar 28 10:50:24 2024 +0530

        Merge branch 'main' into float32

    commit e28335c
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Wed Mar 27 16:33:56 2024 +0530

        Fix integration test failures for TextImportPipelineIT

    commit 8480654
    Merge: c6364b7 afd1dc2
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Wed Mar 27 16:25:57 2024 +0530

        Merge branch 'float32' of github.com:arawind/DataflowTemplates into float32

    commit c6364b7
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Wed Mar 27 16:22:27 2024 +0530

        Add avro integration tests for FLOAT32 type in Spanner.

        This fixes the ExportPipelineIT and adds support for the
        ImportPipelineIT.

    commit afd1dc2
    Merge: eca6acf 4536653
    Author: Aravind Pedapudi <aravindp1510@gmail.com>
    Date:   Wed Mar 27 11:37:24 2024 +0530

        Merge branch 'main' into float32

    commit eca6acf
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Fri Mar 22 17:12:35 2024 +0530

        Address review comments

        Adds new tests in ExportPipelineIT and also removes the INT32 -> FLOAT32 parsing as we don't see any use for it.

    commit c1cd281
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Thu Mar 21 18:15:05 2024 +0530

        Fix Beam to handle float32 arrays and pg schema.

        This was missed in the earlier commits

    commit 5e936f4
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Thu Mar 21 13:32:01 2024 +0530

        Address review comments

    commit f4ab908
    Merge: a6aba30 1df7e0a
    Author: Aravind Pedapudi <aravindp1510@gmail.com>
    Date:   Thu Mar 21 12:56:09 2024 +0530

        Merge branch 'GoogleCloudPlatform:main' into float32

    commit a6aba30
    Merge: 66c488c f44f7ce
    Author: Aravind Pedapudi <aravindp1510@gmail.com>
    Date:   Wed Mar 20 16:21:11 2024 +0530

        Merge branch 'main' into float32

    commit 66c488c
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Wed Mar 20 15:49:31 2024 +0530

        Fix gax dependency failures in spanner integration tests.

        We were seeing this error in the spanner integration tests: `class com.google.cloud.spanner.v1.stub.SpannerStubSettings overrides final method com.google.api.gax.rpc.StubSettings.getEndpoint()Ljava/lang/String;`, which were triggered by the google-cloud-spanner BOM update to 6.61.
        We're fixing these errors by setting the gax version to the one used by Cloud Spanner client libs. This should be removed once Beam is updated to the latest version of gax (which may happen when Beam's dependency on Spanner is updated).

    commit 90c1fbc
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Tue Mar 19 19:17:03 2024 +0530

        Support FLOAT32 type in v1 spanner templates

    commit 51f75f0
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Tue Mar 19 19:10:06 2024 +0530

        Modify the local copy of Beam to support FLOAT32 type.

    commit f85fd00
    Author: Aravind <aravindp1510@gmail.com>
    Date:   Tue Mar 19 18:53:17 2024 +0530

        Update the version of google-cloud-spanner-bom to 6.61.0

Upgrade Beam version to 2.55.1

Specify venv

single commit off of master

No-op comment change to kick copybara

Revert "No-op comment change to kick copybara"

This reverts commit 928dc6b.

Different no-op to kick copybara and avoid conflicts

PR GoogleCloudPlatform#1376: Support FLOAT32 type in v1 templates of Spanner

Please approve this CL. It will be submitted automatically, and its GitHub pull request will be marked as merged.

Imported from GitHub PR GoogleCloudPlatform#1376

GoogleSQL:
* [Export](https://console.cloud.google.com/dataflow/jobs/us-central1/2024-03-21_05_01_22-9618753751624897563;graphView=0?project=span-cloud-testing&pageState=(%22dfTime%22:(%22l%22:%22dfJobMaxTime%22)))
* [Import](https://console.cloud.google.com/dataflow/jobs/us-central1/2024-03-21_05_38_00-8680174928377892705;graphView=0?project=span-cloud-testing&pageState=(%22dfTime%22:(%22l%22:%22dfJobMaxTime%22)))

PG:
* [Export](https://console.cloud.google.com/dataflow/jobs/us-central1/2024-03-21_05_47_07-2771163877050158940;graphView=0?project=span-cloud-testing&pageState=(%22dfTime%22:(%22l%22:%22dfJobMaxTime%22)))
* [Import](https://console.cloud.google.com/dataflow/jobs/us-central1/2024-03-21_05_58_11-1643174307354839989;graphView=0?project=span-cloud-testing&pageState=(%22dfTime%22:(%22l%22:%22dfJobMaxTime%22)))

GoogleSQL:
* [Export](https://console.cloud.google.com/dataflow/jobs/us-central1/2024-03-22_00_17_56-5509502875882896332?e=13802955&mods=monitoring_api_prod&project=span-cloud-testing)
* [Import](https://console.cloud.google.com/dataflow/jobs/us-central1/2024-03-22_02_50_39-14326646282971458157?e=13802955&mods=monitoring_api_prod&project=span-cloud-testing)

PG:
* [Export](https://console.cloud.google.com/dataflow/jobs/us-central1/2024-03-22_03_17_13-5127588554893435640;graphView=0?project=span-cloud-testing&e=13802955&mods=monitoring_api_prod&pageState=(%22dfTime%22:(%22l%22:%22dfJobMaxTime%22)))
* [Import](https://console.cloud.google.com/dataflow/jobs/us-central1/2024-03-22_03_09_55-9477941764979866789?e=13802955&mods=monitoring_api_prod&project=span-cloud-testing)

Copybara import of the project:

  - 357481bb0fdfba9c91fdde1c24ae4bbcaf5d2bf4 Support FLOAT32 type in v1 templates of Spanner. by Aravind <aravindp1510@gmail.com>

COPYBARA_INTEGRATE_REVIEW=GoogleCloudPlatform#1376 from arawind:float32 357481bb0fdfba9c91fdde1c24ae4bbcaf5d2bf4
PiperOrigin-RevId: 622232578

String change to match external

PiperOrigin-RevId: 622247573

Add schemaMapperInterface and implementations

Add namespace to params

spotless

Add user-agent string to Elasticsearch templates

Rename templateName to userAgent

Fix style

Capitalize header name

Add unit tests

Fix style

Bugfix for UI issue removing flex templates from drop down menu

Use beam version defined in the pom.xml to make sure we stay on the most current version. Also add a comment regarding the hashed file location for context.

Implement enum based experiment value provider

disable YamlTemplateIT's until GoogleCloudPlatform#1405 is merged

Signed-off-by: Jeffrey Kinard <jeff@thekinards.com>

Add smoke tests for the no UDF case for Xlang templates, abstract conversion functions, fix bug with flexContainerName.

A small simplification in Bulk Reader tests.

Allow manual triggering of PR workflow

Add python version to setup-env actions

Add parentName and parentTriggerValues attributes

Test that new attributes are present

Remove redundant line
copybara-service bot pushed a commit that referenced this pull request Apr 19, 2024
Please approve this CL. It will be submitted automatically, and its GitHub pull request will be marked as merged.

Imported from GitHub PR #1413

removed deprecated reverse replication templates

Copybara import of the project:

  - 2c59830 removed deprecated reverse replicaiton templates by Akshara Uke <aksharau@google.com>

COPYBARA_INTEGRATE_REVIEW=#1376 from arawind:float32 357481bb0fdfba9c91fdde1c24ae4bbcaf5d2bf4
PiperOrigin-RevId: 626357039
fozzie15 added a commit that referenced this pull request Apr 19, 2024
Add Python UDF support for Elastic Search Templates.

Add parentName and parentTriggerValues attributes

Test that new attributes are present

Remove redundant line

fixed escape characters and timer type

YamlTemplate speed improvements

Signed-off-by: Jeffrey Kinard <jeff@thekinards.com>

rebase

Add python version to setup-env actions

Add parentName and parentTriggerValues attributes

Test that new attributes are present

Remove redundant line

fixed escape characters and timer type

YamlTemplate speed improvements

Signed-off-by: Jeffrey Kinard <jeff@thekinards.com>

PR #1413: removed deprecated reverse replication templates

Please approve this CL. It will be submitted automatically, and its GitHub pull request will be marked as merged.

Imported from GitHub PR #1413

removed deprecated reverse replication templates

Copybara import of the project:

  - 2c59830 removed deprecated reverse replicaiton templates by Akshara Uke <aksharau@google.com>

COPYBARA_INTEGRATE_REVIEW=#1376 from arawind:float32 357481bb0fdfba9c91fdde1c24ae4bbcaf5d2bf4
PiperOrigin-RevId: 626357039
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Google LGTM Approval of a pull request to be merged into the repository size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants