amundsen icon indicating copy to clipboard operation
amundsen copied to clipboard

perf: Neo4j csv publisher using apoc library for performance improvements

Open zacr opened this issue 3 years ago • 7 comments
trafficstars

Summary of Changes

New Neo4JCsvPublisher impementation using the APOC library for 5x performance improvement.

Tests

I didn't see any automated tests for Neo4JCsvPublisher. This PR was tested manually by several community members.

Documentation

CheckList

Make sure you have checked all steps below to ensure a timely review.

  • [ ] PR title addresses the issue accurately and concisely. Example: "Updates the version of Flask to v1.0.2"
  • [ ] PR includes a summary of changes.
  • [ ] PR adds unit tests, updates existing unit tests, OR documents why no test additions or modifications are needed.
  • [ ] In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain docstrings that explain what it does

zacr avatar Apr 28 '22 11:04 zacr

Congratulations on your first Pull Request and welcome to Amundsen community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/amundsen-io/amundsen/blob/main/CONTRIBUTING.md)

boring-cyborg[bot] avatar Apr 28 '22 11:04 boring-cyborg[bot]

this is good! thanks for the contribution. cc @allisonsuarez @dkunitsk wonder whether you two could test it out with lyft env

feng-tao avatar Apr 28 '22 15:04 feng-tao

@feng-tao this requires APOC to be installed (which it isn't by default on the docker-compose). Did you want me to create a PR with the updated docker compose files to include APOC by default?

VoLKyyyOG avatar Apr 29 '22 00:04 VoLKyyyOG

@akiratwang Oops, I hope i didn't mess anything up. I followed the instructions here: https://github.com/amundsen-io/amundsen/pull/1836/checks but maybe I shouldn't have.

zacr avatar May 03 '22 13:05 zacr

could you rebase with master?

feng-tao avatar May 08 '22 03:05 feng-tao

@feng-tao I started to rebase but there were so many merge conflicts and a few of them I wasn't sure about. Since this PR is just one file, maybe I should just create a new branch off latest master and create a PR there?

zacr avatar May 10 '22 12:05 zacr

@feng-tao @akiratwang I fixed up my fork of amundsen and re-created this PR on a clean main branch. https://github.com/amundsen-io/amundsen/pull/1877

zacr avatar May 27 '22 21:05 zacr

@zacr could you sign your commits here?

Golodhros avatar Jan 09 '23 19:01 Golodhros

Closing as dup of https://github.com/amundsen-io/amundsen/pull/1877

Golodhros avatar Feb 01 '23 18:02 Golodhros