Skip to content

fix: psycopg2 Import Handling in PostgresIndex #576

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

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

Conversation

m0n0x41d
Copy link

@m0n0x41d m0n0x41d commented Apr 4, 2025

Summary of Changes

  1. Fixed the import of psycopg2 by moving it from the PostgresIndex class definition to the module level with proper TYPE_CHECKING guards
  2. Removed redundant import attempt inside the class definition that was failing because imports in class bodies execute at class definition time, not instance creation
  3. Updated CONTRIBUTION.md to recommend uv sync --extra all instead of uv pip install -e .[dev] for development environment setup, ensuring all dependencies (including postgres) are properly installed

Notes

  • I did not bump the package version as I am not a maintainer of this repository (version updates should be handled by the maintainers)
  • I did not add tests for this change since the repository currently does not have tests for the indexes functionality

m0n0x41d added 2 commits April 4, 2025 14:53
Move import from class definition to module level with proper type checking.
Class-level import was executing at class definition time, not instantiation.
@jamescalam jamescalam changed the title Fix psycopg2 Import Handling in PostgresIndex fix: psycopg2 Import Handling in PostgresIndex Apr 14, 2025
@jamescalam
Copy link
Member

jamescalam commented Apr 14, 2025

@m0n0x41d I don't think we can move the import from the class definition to the top-level file as it means we will see this error https://github.com/aurelio-labs/semantic-router/actions/runs/14444373527/job/40501518537?pr=576#step:6:32 will occur whenever someone tries to use the lib without postgres installed

Other changes to contributing guidelines are good

@m0n0x41d
Copy link
Author

@jamescalam Hello, thank you for taking a look here.

I am incredibly sorry for being blind to it; you are right that we can't break an import of a base library when the optional dependency is not installed.

However, as I wrote in the related issue, the import in the instance constructor is simply not working because of Python's namespace nature.

I've pushed a small fix, moving an exception raise into init, based on a simple flag we are setting in the try/except import. Please take a look and run tests when you have spare time.

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