-
Notifications
You must be signed in to change notification settings - Fork 27
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
fix #246 #248
base: master
Are you sure you want to change the base?
fix #246 #248
Conversation
@@ -337,6 +337,7 @@ def pydantic_model_optional(self) -> t.Type[pydantic.BaseModel]: | |||
include_default_columns=True, | |||
all_optional=True, | |||
model_name=f"{self.table.__name__}Optional", | |||
**self.schema_extra, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the slow reply and sorry if I missed your point and doing something wrong, but this will break the PATCH
request. I tried something like this
class Message(Table, db=DB):
message_title = Varchar(length=100)
message_content = Varchar()
app = FastAPI(debug=True)
def to_camel(string: str) -> str:
string_split = string.split("_")
return string_split[0] + "".join(
word.capitalize() for word in string_split[1:]
)
class MyConfig(pydantic.BaseConfig):
alias_generator = to_camel
allow_population_by_field_name = True
FastAPIWrapper(
root_url="/message/",
fastapi_app=app,
piccolo_crud=PiccoloCRUD(
table=Message,
read_only=False,
schema_extra={"pydantic_config_class": MyConfig},
),
)
and I get a 400
error on the PATCH
request Unrecognized keys - {'messageContent', 'messageTitle'}
(using curl
) because the camelCase
field is not recognized.
Also PUT
request get 500
error (with that Pydantic configuration of camelCase
in schema_extra
) with mesage AttributeError: type object 'Message' has no attribute 'messageTitle'
Can you confirm that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't worry, better late than never xD. Yes, I saw. I didn't quite understand these 3 data variables/params in the following lines:
piccolo_api/piccolo_api/crud/endpoints.py
Lines 1104 to 1132 in e2f8ce8
@apply_validators | |
@db_exception_handler | |
async def put_single( | |
self, request: Request, row_id: PK_TYPES, data: t.Dict[str, t.Any] | |
) -> Response: | |
""" | |
Replaces an existing row. We don't allow new resources to be created. | |
""" | |
cleaned_data = self._clean_data(data) | |
try: | |
model = self.pydantic_model(**cleaned_data) | |
except ValidationError as exception: | |
return Response(str(exception), status_code=400) | |
cls = self.table | |
values = { | |
getattr(cls, key): getattr(model, key) for key in data.keys() | |
} | |
try: | |
await cls.update(values).where( | |
cls._meta.primary_key == row_id | |
).run() | |
return Response(status_code=204) | |
except ValueError: | |
return Response("Unable to save the resource.", status_code=500) |
cleaned_data
data
model
If I change the code like below we can achieve something:
values = {
getattr(cls, key): getattr(model, key) for key in model.dict().keys()
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will try to explain (as I understand it) and I hope it will be clear enough :).
cleaned_data - payload handling null value (convert null as string in payload to
None
)
data - payload data
model - Pydantic model
The tests fail on the PATCH
request (partial update) because data
dict are not the same as model.dict()
, because the model uses pydantic_model_optional which puts the optional fields as None
and if we patch only one field we need data from payload
not from model
.
Example of payload data:
{'name': 'Star Wars: A New Hope'} <-- data dict
{'id': None, 'name': 'Star Wars: A New Hope', 'rating': None} <-- model.dict()
and because of that we get error 422
in patch
tests if we use
values = {
getattr(cls, key): getattr(model, key) for key in model.dict().keys()
}
passed the
schema_extra
parameter to each method wherecreate_pydantic_model
is called in PiccoloCRUD