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

Documentation for migrating schemas that are nested external to the head package #191

Open
gabloe opened this issue May 31, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@gabloe
Copy link

gabloe commented May 31, 2024

Is your feature request related to a problem? Please describe.
I am evaluating Cadwyn as an option for my API to enable versioning. Many of my application routes use a paginated response model using FastAPI-Pagination, and in these cases the response model is wrapped with / nested in a "Page" model that exists outside of my head schemas package. In my route I will query for data using Beanie, and then project the response.

Consider the below demo project:

demo.schemas.head.customer.py:

def CustomerBase(BaseModel):
   name: str = Field(...)
   address: str = Field(...)

def CustomerIn(CustomerBase):
   pass

def CustomerOut(CustomerBase):
   pass

demo.routes.customer.py:

from demo.models import Customer    # Beanie document model
from demo.exceptions import CustomerNotFoundError
from demo.schemas.head import CustomerIn, CustomerOut
from cadwyn import VersionedAPIRouter
from beanie import PydanticObjectId
from fastapi_pagination import Page

router = VersionedAPIRouter(
    prefix="/customer",
    tags=["customer"],
)

@router.get(
    "/",
    response_model=Page[CustomerOut],
    description="List customers.",
)
async def list_customers(
    customer: CustomerIn,
):
    return await paginate(Customer)

@router.get(
    "/{id}",
    response_model=CustomerOut,
    description="Get customers by ID.",
)
async def get_customer(
    id: PydanticObjectId,
):
    cus = await Customer.get(id)
    if cus is None:
       raise CustomerNotFoundError()
    return customer.model_dump()

I wanted to get an understanding about version migrations so I made a very simple change to the CustomerBase schema by adding an address field, and I wanted to test whether I could enforce that field to be present only in newer API versions. I added a address field to my CustomerBase in the head package, and added a version change to specify that the field did not previously exist.

demo.versions.v2024_05_31.py:

from cadwyn.structure import VersionChange, schema
from demo.schemas.head import CustomerBase


class AddressDidntExist(VersionChange):
    description = "..."
    instructions_to_migrate_to_previous_version = (
        schema(CustomerBase).field("address").didnt_exist,
    )

This works fine for the get_customer route. In the older API version the address is not present in the response, and in the newer version the address is present in the response. But, the list_customers route which uses a wrapped response model includes the address field in both versions.

My assumption here is that because my version change migration is applied to CustomerBase and not Page[CustomerBase] the migration is not getting applied.

The paginated response in the old version looks like:

{
   "items": [
      {
         "id": "665909337b970ec73402d0ed",
         "name": "John Doe",
         "address": "123 Cherry Lane"
      }
   ],
   "total": 1,
   "page": 1,
   "size": 10,
   "pages": 1
}

And when using the get_customer route (non-paginated) I see the expected response after migration.

{
   "id": "665909337b970ec73402d0ed",
   "name": "John Doe"
}

My question is whether this type of migration is supported with Cadwyn? Is it possible to provide migration instructions for schemas that are wrapped with another schema (in this case the Page) which exist outside of my head package?

@gabloe gabloe added the enhancement New feature or request label May 31, 2024
@zmievsa
Copy link
Owner

zmievsa commented Jun 1, 2024

I am afraid that the migration of internal schemas within external schemas is not supported yet. In fact, migrating generic response models might also be hard right now. Additionally, adding an address to the response is not a breaking change so I would recommend to just add it into all versions.

I am currently drafting a feature that would add support for external schemas but it won't be ready any time soon, sadly. I recommend implementing those classes internally

@gabloe
Copy link
Author

gabloe commented Jun 1, 2024

Ok, got it. Yeah I know it’s not a breaking change. I was just doing a very simple test to evaluate the capabilities of Cadwyn.

@zmievsa
Copy link
Owner

zmievsa commented Jun 1, 2024

I'll try to push it forward in my backlog to speed up the process

@zmievsa
Copy link
Owner

zmievsa commented Jun 15, 2024

It's the second thing in my backlog. So if you are willing to wait a little bit, I think it will actually happen relatively soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants