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.

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

Use built-in batch grouping transform #1418

Merged
merged 2 commits into from May 22, 2024

Conversation

fbiville
Copy link
Collaborator

@fbiville fbiville commented Apr 8, 2024

Fixes #1416

Copy link

codecov bot commented Apr 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 40.87%. Comparing base (8dfb6de) to head (46e762c).
Report is 54 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1418      +/-   ##
============================================
- Coverage     40.93%   40.87%   -0.06%     
+ Complexity     2808     2798      -10     
============================================
  Files           739      737       -2     
  Lines         42855    42828      -27     
  Branches       4582     4579       -3     
============================================
- Hits          17542    17508      -34     
- Misses        23814    23823       +9     
+ Partials       1499     1497       -2     
Components Coverage Δ
spanner-templates 59.85% <ø> (ø)
spanner-import-export 65.59% <ø> (ø)
spanner-live-forward-migration 73.67% <ø> (ø)
spanner-live-reverse-replication 48.64% <ø> (ø)
spanner-bulk-migration 78.01% <ø> (ø)
Files Coverage Δ
...ogle/cloud/teleport/v2/neo4j/model/job/Config.java 68.75% <100.00%> (ø)
...t/v2/neo4j/transforms/Neo4jRowWriterTransform.java 69.69% <100.00%> (-0.46%) ⬇️

... and 1 file with indirect coverage changes

@fbiville fbiville marked this pull request as ready for review April 8, 2024 15:04
@fbiville fbiville force-pushed the neo4j/gh-1416 branch 3 times, most recently from a51a20a to a228987 Compare April 12, 2024 08:02
Copy link
Contributor

@ali-ince ali-ince left a comment

Choose a reason for hiding this comment

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

LGTM

@ali-ince
Copy link
Contributor

@liferoad I think this is ready for you to review.

@liferoad
Copy link
Contributor

@fbiville
Copy link
Collaborator Author

Can you check the failed tests? https://github.com/GoogleCloudPlatform/DataflowTemplates/pull/1418/checks?check_run_id=25124983697

@liferoad ConstraintsIndicesIT$Neo4j5EnterpriseIT.canResetDatabase needs to be muted until the more recent Neo4jResourceManager API is available and used (maybe this branch has not been rebased in a while?)
The other test seems unrelated to the template.

@liferoad
Copy link
Contributor

Can you rebase it?

This does not change anything in practice, since edge and custom
query targets share the same default value.
@fbiville
Copy link
Collaborator Author

@liferoad now rebased

@liferoad
Copy link
Contributor

https://github.com/GoogleCloudPlatform/DataflowTemplates/actions/runs/9171539329?pr=1418

The job running on runner nokill-gitactions-runner-7jhd has exceeded the maximum execution time of 180 minutes.

Is this caused by the new tests you added?
 

@liferoad liferoad added the Google LGTM Approval of a pull request to be merged into the repository label May 21, 2024
@copybara-service copybara-service bot merged commit 9b5416f into GoogleCloudPlatform:main May 22, 2024
13 of 14 checks passed
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/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Google_cloud_to_neo4j v2024-03-27-00_rc00 causes unexpected increase of shuffle data processed and worker OOMs
3 participants