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

Alembic migrations working with argilla as Python package #2511

Conversation

jfcalvo
Copy link
Member

@jfcalvo jfcalvo commented Mar 9, 2023

This PR include changes to allow alembic database migrations work as expected with argilla built as a Python package.

I have moved alembic.ini from root project to src/argilla/alembic.ini . With this change we can't execute alembic anymore without specifying where alembic.ini is with --config parameter.

So in order to execute alembic command in development we need to set --config parameter:

$ alembic --config src/argilla/alembic.ini upgrade head

Alternatively we can set ALEMBIC_CONFIG:

$ ALEMBIC_CONFIG=src/argilla/alembic.ini alembic upgrade head

I followed the steps below to test the changes:

Create a testing environment to build the argilla package

$ conda create -n argilla-testing python=3.8
$ conda activate argilla-testing

Build the package

Only building the python code for testing purposes:

$ python -m build

Install the package

$ pip install dist/argilla-1.4.0.dev0.tar.gz[server,listeners,postgresql]

Testing that changes are working

These command should be executed outside argilla root project path, to be sure that everything is working running with the installed Python package.

Running database migrations (default SQLite):

$ python -m argilla.tasks.database.migrate
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade  -> 74694870197c, create users table
INFO  [alembic.runtime.migration] Running upgrade 74694870197c -> 82a5a88a3fa5, create workspaces table
INFO  [alembic.runtime.migration] Running upgrade 82a5a88a3fa5 -> 1769ee58fbb4, create workspaces_users table

Running database migrations with PostreSQL:

$ createdb argilla
$ ARGILLA_DATABASE_URL=postgresql://postgres:postgres@localhost:5432/argilla python -m argilla.tasks.database.migrate
INFO  [alembic.runtime.migration] Context impl PostgresqlImpl.
INFO  [alembic.runtime.migration] Will assume transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade  -> 74694870197c, create users table
INFO  [alembic.runtime.migration] Running upgrade 74694870197c -> 82a5a88a3fa5, create workspaces table
INFO  [alembic.runtime.migration] Running upgrade 82a5a88a3fa5 -> 1769ee58fbb4, create workspaces_users table

Adding user with default credentials (default SQLite):

$ python -m argilla.tasks.users.create_default
User with default credentials succesfully created:
• username: 'argilla'
• password: '1234'
• api_key:  'argilla.apikey'

Adding user with default credentials with PostgreSQL:

$ ARGILLA_DATABASE_URL=postgresql://postgres:postgres@localhost:5432/argilla python -m argilla.tasks.users.create_default
User with default credentials succesfully created:
• username: 'argilla'
• password: '1234'
• api_key:  'argilla.apikey'

@jfcalvo jfcalvo requested a review from frascuchon March 9, 2023 10:34
@frascuchon frascuchon merged commit 463f244 into feat/add-basic-database-support Mar 9, 2023
@frascuchon frascuchon deleted the alembic-migrations-with-python-package branch March 9, 2023 15:27
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