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

Removes unneeded ImportErrors #50

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

alexthomas93
Copy link
Collaborator

@alexthomas93 alexthomas93 commented Feb 26, 2025

Description

  • Removes unneeded try: import neo4j statements.
  • Removes tests which test for errors raised when these statements fail.
  • Adds clear_neo4j_database and neo4j_credentials fixtures to the integration tests streamlining handling of database credentials in these tests and making them less flaky.
  • Updates integration tests to use these fixtures.

Type of Change

  • New feature
  • Bug fix
  • Breaking change
  • Project configuration change

Complexity

Low

How Has This Been Tested?

  • Unit tests
  • Integration tests
  • Manual tests

Checklist

  • Unit tests updated
  • Integration tests updated
  • CHANGELOG.md updated

@alexthomas93 alexthomas93 changed the title Updated CHANGELOG Removes unneeded ImportErrors Feb 26, 2025
@alexthomas93 alexthomas93 requested review from stellasia and willtai and removed request for stellasia February 26, 2025 15:47
@alexthomas93 alexthomas93 marked this pull request as ready for review February 26, 2025 15:47
url=url,
username=username,
password=password,
url=neo4j_credentials["url"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Small detail, but I think:

Suggested change
url=neo4j_credentials["url"],
graph = Neo4jGraph(**neo4j_credentials)

could save us 0.1 seconds in the future :-P (OK we've invented copy/paste, I know ^^)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants