Error applying migration m008_reply_to_id_foreign_key #162
Labels
bug
Something isn't working
Comments
kevinschaul
added a commit
to kevinschaul/llm
that referenced
this issue
Aug 18, 2023
The tests currently fail, due to migration m008_reply_to_id_foreign_key failing. I suspect this is related to a recent change in squlite-utils [0]. This PR avoids that by explictly dropping the foreign key before renaming the table, then adding the foreign key back. See the related GitHub issue for more info [1]. [0] simonw/sqlite-utils#577 [1] simonw#162
kevinschaul
added a commit
to kevinschaul/llm
that referenced
this issue
Aug 18, 2023
The tests currently fail, due to migration m008_reply_to_id_foreign_key failing. I suspect this is related to a recent change in squlite-utils [0]. This PR avoids that by explictly dropping the foreign key before renaming the table, then adding the foreign key back. Fixes simonw#162 [0] simonw/sqlite-utils#577
|
I'm having trouble reproducing this one. Can you provide steps to reproduce? |
|
Of course. I am using OS X 13.5, python 3.10.4 via pyenv and pipx. First, remove llm, and importantly the db file: Then install it. I also install llm-gpt4all because I don't have an OpenAI API key. Then try a prompt. The prompt successfully runs, but llm errors out when it attempts to log to the sqlite database: Inspecting logs.db shows that migration m007_finish_logs_table was applied but none that follow.
|
|
I tried to recreate in a fresh virtual environment like this: cd /tmp
mkdir llm-reproduce
cd llm-reproduce
python -m venv venv
source venv/bin/activateThen in the virtual environment: pip install llm
llm install llm-gpt4all
LLM_USER_PATH=. llm -m 'orca-mini-3b' 'say hi'The Output was: And LLM_USER_PATH=. llm logs[
{
"id": "01h87kcadjbd8tcdean8ej71cd",
"model": "orca-mini-3b",
"prompt": "say hi",
"system": null,
"prompt_json": null,
"options_json": {},
"response": " Hello! I'm here to assist you in any way possible. How may I help you today?",
"response_json": {
"full_prompt": "### System:\nYou are an AI assistant that follows instruction extremely well. Help as much as you can.\n\n\n### User:\nsay hi\n### Response:\n"
},
"conversation_id": "01h87kcadjsn68854fy8jtg4ha",
"duration_ms": 6706,
"datetime_utc": "2023-08-19T19:11:55.737818",
"conversation_name": "say hi",
"conversation_model": "orca-mini-3b"
}
]I'm really keen to duplicate this bug, because if it's a Could you run this one-liner and share the results? python -c '
import sqlite3
import sys
conn = sqlite3.connect(":memory:")
print("Python version:", sys.version)
print("SQLite version:", conn.execute("select sqlite_version()").fetchone()[0])
print("SQLite compile options\n " + "\n ".join(r[0] for r in conn.execute("pragma compile_options").fetchall()))
for pragma in (
"foreign_keys", "defer_foreign_keys", "ignore_check_constraints", "legacy_alter_table",
"recursive_triggers", "writable_schema",
):
output = conn.execute("pragma %s" % pragma).fetchone()
print("Pragma {}: {}".format(pragma, output[0]))
'Here's what I get in that |
|
|
Aha! |
|
No, that didn't recreate the bug for me: >>> import llm
>>> from sqlite_utils import Database
>>> db = Database(memory=True)
>>> db.execute('pragma writable_schema').fetchall()
[(0,)]
>>> db.execute('pragma writable_schema=1')
<sqlite3.Cursor object at 0x10676a7c0>
>>> db.execute('pragma writable_schema').fetchall()
[(1,)]
>>> from llm.migrations import migrate
>>> migrate(db)Could you try running this script and see what result you get? from llm.migrations import migrate
from sqlite_utils import Database
db = Database(memory=True)
migrate(db)
print(db.schema)Or in more convenient copy-paste format: python -c '
from llm.migrations import migrate
from sqlite_utils import Database
db = Database(memory=True)
migrate(db)
print(db.schema)
' |
|
First attempt: After a |
|
Aha! Now that's the bug which was fixed in |
|
Ah interesting. Looks like I am on |
|
What happens if you do this: pip install -U sqlite-utils
python -c '
from llm.migrations import migrate
from sqlite_utils import Database
db = Database(memory=True)
migrate(db)
print(db.schema)
' |
|
|
OK! Sot it looks like, on your system, with the most recent python -c '
from llm.migrations import migrate
from sqlite_utils import Database
db = Database(memory=True)
migrate(db)
print(db.schema)
'But on my system it does not. |
|
"python 3.10.4 via pyenv" - I'm going to try that. |
|
Yes! I have replicated the bug. Using this pattern: https://til.simonwillison.net/python/quick-testing-pyenv pyenv install 3.10.4Then to create a venv with it: ~/.pyenv/versions/3.10.4/bin/python -m venv venv310
source venv310/bin/activate
pip install llmAnd now: Gives me this error: |
|
My environment dump: |
|
FWIW turning off |
|
Yes! That worked for me too. OK, so we know the root cause now. Thanks for helping me find that. Next I need to figure out if this should be fixed just in |
|
Excellent! Thank you |
|
Here's a recreation that doesn't depend on python -c '
import sqlite_utils
db = sqlite_utils.Database(memory=True)
db["log"].create(
{
"provider": str,
"system": str,
"prompt": str,
"chat_id": str,
"response": str,
"model": str,
"timestamp": str,
}
)
db["log"].transform(pk="id")
db["log"].transform(types={"chat_id": int})
db["log"].add_foreign_key("chat_id", "log", "id")
db["log"].transform(
column_order=(
"id",
"model",
"timestamp",
"prompt",
"system",
"response",
"chat_id",
)
)
db["log"].transform(drop=("provider",))
db["log"].add_column("debug", str)
db["log"].add_column("duration_ms", int)
columns = db["log"].columns_dict
for column, type in (
("options_json", str),
("prompt_json", str),
("response_json", str),
("reply_to_id", int),
):
if column not in columns:
db["log"].add_column(column, type)
# Use .transform() to rename options and timestamp_utc, and set new order
db["log"].transform(
column_order=(
"id",
"model",
"prompt",
"system",
"prompt_json",
"options_json",
"response",
"response_json",
"reply_to_id",
"chat_id",
"duration_ms",
"timestamp_utc",
),
rename={
"timestamp": "timestamp_utc",
"options": "options_json",
},
)
db["log"].transform(
drop={"debug"},
rename={"timestamp_utc": "datetime_utc"},
)
with db.conn:
db.execute("alter table log rename to logs")
# This is where it breaks:
db["logs"].add_foreign_key("reply_to_id", "logs", "id")
' |
|
And a simplified version of that: python -c '
import sqlite_utils
db = sqlite_utils.Database(memory=True)
db.execute("""
CREATE TABLE "logs" (
[id] INTEGER PRIMARY KEY,
[model] TEXT,
[prompt] TEXT,
[system] TEXT,
[prompt_json] TEXT,
[options_json] TEXT,
[response] TEXT,
[response_json] TEXT,
[reply_to_id] INTEGER,
[chat_id] INTEGER REFERENCES [log]([id]),
[duration_ms] INTEGER,
[datetime_utc] TEXT
);
""")
# This is where it breaks:
db["logs"].add_foreign_key("reply_to_id", "logs", "id")
' |
|
On further thought, I'm going to treat this as a bug in The simplified STR here illustrates why: python -c '
import sqlite_utils
db = sqlite_utils.Database(memory=True)
db.execute("""
CREATE TABLE "logs" (
[id] INTEGER PRIMARY KEY,
[chat_id] INTEGER REFERENCES [log]([id]),
[reply_to_id] INTEGER
);
""")
db["logs"].add_foreign_key("reply_to_id", "logs", "id")
'That The reason it's hurting us now is that in In 3.35 it changed to using |
simonw
added a commit
that referenced
this issue
Aug 19, 2023
|
OK, I think this is a good fix. Could you test this fix for me and see if it works? You can install a working version using: pipx uninstall llmpipx install https://github.com/simonw/llm/archive/c4dbb62e68e56f1208927064cf374a00dcd643b2.zipThat should give you a Does that fix the problem? If so I'll ship a bug fix release. |
simonw
added a commit
that referenced
this issue
Aug 19, 2023
|
I also had to delete my db file, but then yes this fixes it! |
|
Great! Shipping it now. |
simonw
added a commit
that referenced
this issue
Aug 19, 2023
simonw
added a commit
that referenced
this issue
Aug 19, 2023
|
0.7.1 bug fix release is out, with these changes: 0.7...0.7.1 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
On a fresh install of
llm, I am running into a sqlite error when runningllm logs status:Best I can tell, this call fails because after renaming the table from
logtologs, a foreign key remains pointing to the outdatedlogtable name.This call:
ends up attempting to alter the table
log, which no longer exists.It may be related to this recent sqlite-utils change. I'm unsure if the bug is in sqlite-utils or llm. Possibly llm needs to adjust the previous foreign key directly?
The text was updated successfully, but these errors were encountered: