Conversation
… coder which rely on decoding the (currently unsupported) custom non-JSON data types
…example code endpoints
github-actions
bot
added
the
changelog-provided
|
@long2ice Gentle nudge? |
rth
left a comment
rth
left a comment
There was a problem hiding this comment.
Thanks @vicchi . A few comments.
Just to clarify do you have commit rights for this project or not?
Regarding tests, yes it's unfortunate that they are failing on main. Given the situation I agree that skipping the failing tests to make main green is probably best. v0.2.2 was released with those failing tests, so at least this shouldn't break anything backward compatibility wise.
I'm trying to make it work with modern version of Python, and indeed there are issue, so a new release would be very welcome.
The rest of the changes LGTM.
|
|
||
| async def get_with_ttl(self, key: str) -> Tuple[int, Optional[bytes]]: | ||
| response = await self.client.get_item(TableName=self.table_name, Key={"key": {"S": key}}) | ||
| if self.client: |
There was a problem hiding this comment.
Better to have
if not self.client:
return 0, None
to avoid deeply nested ifs and changing the indention of everthing below.
| - (dependabot) Bump actions/upload-artifact from 3 to 4 (#360) [#360](https://github.com/long2ice/fastapi-cache/issues/360) | ||
| - (dependabot) Bump actions/cache from 3 to 4 (#378) [#378](https://github.com/long2ice/fastapi-cache/issues/378) | ||
| - (dependabot) Bump dependabot/fetch-metadata from 1 to 2 (#464) [#464](https://github.com/long2ice/fastapi-cache/issues/464) | ||
| - (dependabot) Bump tox from 4.20.0 to 4.23.2 (#466) [#466](https://github.com/long2ice/fastapi-cache/issues/466) |
There was a problem hiding this comment.
It's a dev dependency, users wouldn't care. Same about CI related updates (actions/download-artifact etc). It might be better to exclude those from the changelog.
| invalid = b'{"name": "incomplete"}' | ||
| with pytest.raises(ValidationError): | ||
| JsonCoder.decode_as_type(invalid, type_=PDItem) | ||
| # def test_json_coder_validation_error() -> None: |
There was a problem hiding this comment.
Maybe better to mark it as a known failure rather than commenting with @pytest.mark.xfail(reason="See issue #460")
2 participants
@long2ice This PR is a maintenance release for v0.2.3. It's mainly to nuke most outstanding dependabot PRs, to ensure the tests pass and to make sure I've grokked the way the project should be maintained and updated.
I'm happy to merge, tag and release this version but would really appreciate your feedback before I do.