-
Notifications
You must be signed in to change notification settings - Fork 585
fix: run cognee in Docker [COG-1961] #775
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
Conversation
Please make sure all the checkboxes are checked:
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces two new Alembic migration scripts: one for the initial database schema creation and another for adding a default user. Each migration defines both upgrade and downgrade functions to apply or revert the changes. Additionally, the Dockerfile has been corrected to properly separate the Changes
Sequence Diagram(s)sequenceDiagram
participant Alembic
participant DB Engine
participant User Model
Alembic->>DB Engine: upgrade() (initial migration)
DB Engine-->>Alembic: Create database schema
Alembic->>User Model: upgrade() (add default user)
User Model-->>Alembic: Create default user
Alembic->>User Model: downgrade() (remove default user)
User Model-->>Alembic: Delete default user
Alembic->>DB Engine: downgrade() (initial migration)
DB Engine-->>Alembic: Drop database schema
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
alembic/versions/482cd6517ce4_add_default_user.py (1)
27-28
: Consider using a configurable email in downgradeThe downgrade function uses a hardcoded email address "default_user@example.com" rather than deriving it from the same configuration source used in
create_default_user()
.-def downgrade() -> None: - await_only(delete_user("default_user@example.com")) +def downgrade() -> None: + from cognee.config import get_base_config + base_config = get_base_config() + default_user_email = base_config.default_user_email or "default_user@example.com" + await_only(delete_user(default_user_email))This ensures the downgrade operation will delete the correct user even if a custom email was configured.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docker-compose.yml
is excluded by!**/*.yml
📒 Files selected for processing (3)
Dockerfile
(1 hunks)alembic/versions/482cd6517ce4_add_default_user.py
(1 hunks)alembic/versions/8057ae7329c2_initial_migration.py
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
alembic/versions/8057ae7329c2_initial_migration.py (3)
cognee/infrastructure/databases/relational/get_relational_engine.py (1)
get_relational_engine
(5-8)alembic/versions/482cd6517ce4_add_default_user.py (2)
upgrade
(23-24)downgrade
(27-28)cognee/infrastructure/databases/relational/sqlalchemy/SqlAlchemyAdapter.py (2)
create_database
(309-318)delete_database
(320-347)
alembic/versions/482cd6517ce4_add_default_user.py (2)
cognee/modules/users/methods/create_default_user.py (1)
create_default_user
(5-19)alembic/versions/8057ae7329c2_initial_migration.py (2)
upgrade
(20-22)downgrade
(25-27)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: End-to-End Tests / Run Telemetry Pipeline Test
- GitHub Check: End-to-End Tests / S3 Bucket Test
- GitHub Check: End-to-End Tests / Deduplication Test
- GitHub Check: Basic Tests / Run Unit Tests
🔇 Additional comments (6)
Dockerfile (1)
28-32
: Improved Docker build reliability by separating apt commandsSeparating
apt-get update
andapt-get install
into distinct RUN instructions is a Docker best practice. This prevents caching issues where an outdated package index might be used for installations, ensuring gcc and libpq-dev (required for PostgreSQL connections) are installed correctly.alembic/versions/8057ae7329c2_initial_migration.py (3)
9-11
: Good use of await_only for async database operationsProperly importing and using SQLAlchemy's
await_only
utility to handle async database operations in Alembic's synchronous context.
20-22
: Well-structured database initializationThe upgrade function correctly retrieves the database engine and asynchronously creates the database schema, wrapped with
await_only
for proper execution in Alembic's synchronous context.
25-27
: Appropriate downgrade implementationThe downgrade function properly reverses the migration by deleting the database, ensuring idempotency in migrations.
alembic/versions/482cd6517ce4_add_default_user.py (2)
17-20
: Proper migration dependency configurationThe migration correctly specifies its dependency on the initial migration by setting both
down_revision
anddepends_on
appropriately.
23-24
: Effective default user creationThe upgrade function properly uses
await_only
to execute the asynchronous default user creation in Alembic's synchronous context.
<!-- .github/pull_request_template.md --> ## Description Resolve issue with .venv being broken when using docker compose with Cognee ## DCO Affirmation I affirm that all code in every commit of this pull request conforms to the terms of the Topoteretes Developer Certificate of Origin. --------- <!-- .github/pull_request_template.md --> ## Description <!-- Provide a clear description of the changes in this PR --> ## DCO Affirmation I affirm that all code in every commit of this pull request conforms to the terms of the Topoteretes Developer Certificate of Origin. Co-authored-by: Igor Ilic <30923996+dexters1@users.noreply.github.com>
Description
Resolve issue with .venv being broken when using docker compose with Cognee
DCO Affirmation
I affirm that all code in every commit of this pull request conforms to the terms of the Topoteretes Developer Certificate of Origin.