Fix 500 error when getting a non-existing project #900

tcompa · 2023-10-13T12:03:47Z

A change that came about with 1.3.12a0 is that the creation of a project does not automatically also create a dataset associated to it.
While testing in fractal-web, I found the following 500 error. I think the way to reproduce it is to GET a non-existing project.

INFO:     127.0.0.1:52984 - "GET /api/v1/project/1 HTTP/1.1" 500 Internal Server Error
ERROR:    Exception in ASGI application
Traceback (most recent call last):
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/uvicorn/protocols/http/h11_impl.py", line 407, in run_asgi
    result = await app(  # type: ignore[func-returns-value]
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/uvicorn/middleware/proxy_headers.py", line 78, in __call__
    return await self.app(scope, receive, send)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/fastapi/applications.py", line 292, in __call__
    await super().__call__(scope, receive, send)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/applications.py", line 122, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/middleware/errors.py", line 184, in __call__
    raise exc
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/middleware/errors.py", line 162, in __call__
    await self.app(scope, receive, _send)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/middleware/cors.py", line 91, in __call__
    await self.simple_response(scope, receive, send, request_headers=headers)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/middleware/cors.py", line 146, in simple_response
    await self.app(scope, receive, send)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/middleware/exceptions.py", line 79, in __call__
    raise exc
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/middleware/exceptions.py", line 68, in __call__
    await self.app(scope, receive, sender)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/fastapi/middleware/asyncexitstack.py", line 20, in __call__
    raise e
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/fastapi/middleware/asyncexitstack.py", line 17, in __call__
    await self.app(scope, receive, send)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/routing.py", line 718, in __call__
    await route.handle(scope, receive, send)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/routing.py", line 276, in handle
    await self.app(scope, receive, send)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/routing.py", line 66, in app
    response = await func(request)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/fastapi/routing.py", line 291, in app
    content = await serialize_response(
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/fastapi/routing.py", line 154, in serialize_response
    raise ResponseValidationError(
fastapi.exceptions.ResponseValidationError: 1 validation errors:
  {'loc': ('response', 'dataset_list', 0, 'history'), 'msg': 'none is not an allowed value', 'type': 'type_error.none.not_allowed'}

The text was updated successfully, but these errors were encountered:

tcompa · 2023-10-13T12:07:06Z

I cannot reproduce this error with a fresh db.
I will check again, trying to first create a DB with 1.3.11 and then migrating to 1.3.12a0.

tcompa · 2023-10-13T12:10:43Z

Here is how to reproduce the error:

Install fractal-server 1.3.11
Run set-db and start
Create a project
Stop fractal-server
Install fractal-server 1.3.12a0
Run set-db

$ fractalctl set-db
Run alembic.config, with argv=['-c', '/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/fractal_server/alembic.ini', 'upgrade', 'head']
fractal_server.app.db
INFO  [alembic.runtime.migration] Context impl SQLiteImpl.
INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade 8f79bd162e35 -> 7a5a07816dd5, Add Dataset.history

GET projects

INFO:     Uvicorn running on http://127.0.0.1:8000 (Press CTRL+C to quit)
INFO:     127.0.0.1:57524 - "GET /auth/whoami HTTP/1.1" 200 OK
INFO:     127.0.0.1:57538 - "GET /api/alive/ HTTP/1.1" 200 OK
INFO:     127.0.0.1:57524 - "GET /auth/whoami HTTP/1.1" 200 OK
INFO:     127.0.0.1:57546 - "GET /api/v1/project/ HTTP/1.1" 500 Internal Server Error
ERROR:    Exception in ASGI application
Traceback (most recent call last):
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/uvicorn/protocols/http/h11_impl.py", line 407, in run_asgi
    result = await app(  # type: ignore[func-returns-value]
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/uvicorn/middleware/proxy_headers.py", line 78, in __call__
    return await self.app(scope, receive, send)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/fastapi/applications.py", line 292, in __call__
    await super().__call__(scope, receive, send)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/applications.py", line 122, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/middleware/errors.py", line 184, in __call__
    raise exc
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/middleware/errors.py", line 162, in __call__
    await self.app(scope, receive, _send)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/middleware/cors.py", line 91, in __call__
    await self.simple_response(scope, receive, send, request_headers=headers)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/middleware/cors.py", line 146, in simple_response
    await self.app(scope, receive, send)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/middleware/exceptions.py", line 79, in __call__
    raise exc
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/middleware/exceptions.py", line 68, in __call__
    await self.app(scope, receive, sender)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/fastapi/middleware/asyncexitstack.py", line 20, in __call__
    raise e
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/fastapi/middleware/asyncexitstack.py", line 17, in __call__
    await self.app(scope, receive, send)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/routing.py", line 718, in __call__
    await route.handle(scope, receive, send)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/routing.py", line 276, in handle
    await self.app(scope, receive, send)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/starlette/routing.py", line 66, in app
    response = await func(request)
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/fastapi/routing.py", line 291, in app
    content = await serialize_response(
  File "/home/tommaso/Fractal/fractal-web/lib/fractal-server/venv/lib/python3.10/site-packages/fastapi/routing.py", line 154, in serialize_response
    raise ResponseValidationError(
fastapi.exceptions.ResponseValidationError: 1 validation errors:
  {'loc': ('response', 0, 'dataset_list', 0, 'history'), 'msg': 'none is not an allowed value', 'type': 'type_error.none.not_allowed'}

tcompa · 2023-10-13T13:15:03Z

TLDR, based on long test session with @mfranzon

We are still unable to set a new column value to some JSON value (e.g. []). The defaults of sqlmodel.Field do matter only when a new object is created, and do not enter in the migration. Modifying the migration script as in

  def upgrade() -> None:
      with op.batch_alter_table("dataset", schema=None) as batch_op:
        batch_op.add_column(sa.Column("history", sa.JSON(), nullable=True, default=list))

does not solve the problem.

Likely we will have to make the attribute optional.

…bute (ref #900)

tcompa · 2023-10-16T08:04:23Z

This was indeed quite subtle.

Whenever we use sqlmodel.Field and we pass a sa_column=.., the information that is used in Alembic when autogenerating the migrations is the one in sa_column. This can be very counterintuitive, when such behavior differs from the one we would expect from sqlmodel.

For instance in

class Dataset(SQLModel, table=True):
    x: str = Field(sa_column=Column(String, server_default="something"))

there is no Optional[str], and therefore we expect sqlmodel to set the x column as non-nullable.
However, this results in the following autogenerated migration

  def upgrade() -> None:
      with op.batch_alter_table('dataset', schema=None) as batch_op:
          batch_op.add_column(sa.Column('x', sa.String(), server_default='something', nullable=True))

The current solution is to provide a value for the nullable argument of Column, which is otherwise True by default:

Defaults to True unless Column.primary_key is also True or the column specifies a Identity, in which case it defaults to False.
https://docs.sqlalchemy.org/en/14/core/metadata.html#sqlalchemy.schema.Column.params.nullable

Therefore the model will look like

class Dataset(SQLModel, table=True):
    x: str = Field(sa_column=Column(String, server_default="something", nullable=False))

High-level: such model definition challenges our choice of using sqlmodel, since in this case we are providing all arguments through sqlalchemy Column. This is a discussion which we'll pick up at some point.

Related sqlmodel issues/PR:

tcompa · 2023-10-16T08:05:42Z

Implemented via 1ea7968.
Notice also the use of server_default:

sa_column=Column(JSON, server_default="[]", nullable=False)

…rror-when-getting-a-non-existing-project Fix migration for `Dataset.history` (ref #900)

jluethi added this to Fractal Project Management Oct 13, 2023

github-project-automation bot moved this to TODO in Fractal Project Management Oct 13, 2023

tcompa added a commit that referenced this issue Oct 13, 2023

Make _DatasetBase.history optional (ref #900)

e9e3898

tcompa linked a pull request Oct 13, 2023 that will close this issue

Fix migration for Dataset.history (ref #900) #901

Merged

1 task

tcompa added the High Priority Current Priorities & Blocking Issues label Oct 13, 2023

tcompa added a commit that referenced this issue Oct 16, 2023

Add nullable=False in model definition, for Dataset.history attri…

1ea7968

…bute (ref #900)

This was referenced Oct 16, 2023

Test new migrations against "old" data #904

Closed

Fix migration for Dataset.history (ref #900) #901

Merged

tcompa closed this as completed in #901 Oct 16, 2023

tcompa added a commit that referenced this issue Oct 16, 2023

Merge pull request #901 from fractal-analytics-platform/900-fix-500-e…

cca1b51

…rror-when-getting-a-non-existing-project Fix migration for `Dataset.history` (ref #900)

github-project-automation bot moved this from TODO to Done in Fractal Project Management Oct 16, 2023

jluethi removed this from Fractal Project Management Apr 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix 500 error when getting a non-existing project #900

Fix 500 error when getting a non-existing project #900

tcompa commented Oct 13, 2023

tcompa commented Oct 13, 2023

Uh oh!

tcompa commented Oct 13, 2023

Uh oh!

tcompa commented Oct 13, 2023

Uh oh!

tcompa commented Oct 16, 2023

Uh oh!

tcompa commented Oct 16, 2023

Uh oh!

Fix 500 error when getting a non-existing project #900

Fix 500 error when getting a non-existing project #900

Comments

tcompa commented Oct 13, 2023

tcompa commented Oct 13, 2023

Uh oh!

tcompa commented Oct 13, 2023

Uh oh!

tcompa commented Oct 13, 2023

Uh oh!

tcompa commented Oct 16, 2023

Uh oh!

tcompa commented Oct 16, 2023

Uh oh!