8000 Fields with `serialization_alias` have trouble being migrated · Issue #260 · zmievsa/cadwyn · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fields with serialization_alias have trouble being migrated #260

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.

Sign up for GitHub

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

Jump to bottom
Open
ashb opened this issue Mar 19, 2025 · 3 comments
Open

Fields with serialization_alias have trouble being migrated #260

ashb opened this issue Mar 19, 2025 · 3 comments

Comments

@ashb
Copy link
Contributor
ashb commented Mar 19, 2025 •
edited
Loading

If I have this model and endpoint:

class VariablePostBody(BaseModel):
    """Request body schema for creating variables."""

   model_config = ConfigDict(from_attributes=True, populate_by_name=True, extra="forbid")

    value: str | None = Field(serialization_alias="val")
    description: str | None = Field(default=None)

@router.put("/{variable_key}", status_code=201)
def put_variable(variable_key: str, body: VariablePostBody): ....

I get this error

FAILED tests/api_fastapi/execution_api/routes/test_variables.py::TestPutVariable::test_overwriting_existing_variable - cadwyn.exceptions.CadwynHeadRequestValidationError: We failed to migrate the request with version=2025-03-19. This means that there is some error in your migrations or schema structure that makes it impossible to migrate the request of that version to latest.
body={'val': 'new_value'}

errors=[
    {
        "type": "missing",
        "loc": [
            "body",
            "value"
        ],
        "msg": "Field required",
        "input": {
            "val": "new_value"
        }
    },
    {
        "type": "extra_forbidden",
        "loc": [
            "body",
            "val"
        ],
        "msg": "Extra inputs are not permitted",
        "input": "new_value"
    }
]

I don't know quite why we're defining the alias like we are, but see original source here https://github.com/apache/airflow/blob/2500dcf20d2782d16da53ee857c0aab21bfdfbf2/airflow/api_fastapi/execution_api/datamodels/variable.py

Just defining them with alias="val" works.
@ashb ashb changed the title Fields with alias have trouble being migrated Fields with serialization_alias have trouble being migrated Mar 19, 2025
@ashb ashb mentioned this issue Mar 20, 2025
Merged
@zmievsa
Copy link
Owner
zmievsa commented Mar 22, 2025

Hm... I'm afraid this one won't be easy to fix. Because currently we extract request body from the body parameter if it exists and directly if it doesn't exist. Body parameter in this case is VariablePostBody. So when we do VariablePostBody().model_dump(by_alias=True), we get a result that is incompatible with the validation of the latest version -- it's definitely a problem. However, if I set by_alias=False, then the case with alias="val" will stop working :)

How urgent is this one? I can rewrite this part of cadwyn to use https://fastapi.tiangolo.com/how-to/custom-request-and-route/ so we will have more control and will be able to extract the actual body but:

  1. It will likely be a big feature
  2. It might be a breaking change but I'll need to look at all the cases to definitively say

So if it's urgent, I can get to it. If not -- will probably have to postpone it for a few weeks/months

@ashb
Copy link
Contributor Author
ashb commented Mar 22, 2025

Not urgent, I changed Airflow to just use alias

@zmievsa
Copy link
Owner
zmievsa commented Mar 22, 2025

Related issue.
#214

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ashb @zmievsa
0