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

handle CantDeserializeException raised from deserialize method #236

Closed
wants to merge 1 commit into from

Conversation

sjhewitt
Copy link
Contributor

continuation of #233

moves the tests to the _GenericSerializerTest class

@zzzeek zzzeek requested a review from sqla-tester April 14, 2023 14:05
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 98fdfcd of this pull request into gerrit so we can run tests and reviews and stuff

@sqla-tester
Copy link
Collaborator

New Gerrit review created for change 98fdfcd: https://gerrit.sqlalchemy.org/c/sqlalchemy/dogpile.cache/+/4569

Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision bb66ff8 of this pull request into gerrit so we can run tests and reviews and stuff

@sqla-tester
Copy link
Collaborator

Patchset bb66ff8 added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/dogpile.cache/+/4569

@zzzeek
Copy link
Member

zzzeek commented Apr 18, 2023

just to double check, the use case here is the unpickler would raise previously and cause a failure, here we have that it tells the get routine "NO VALUE" so it looks like the value is not in the cache, is that right?

how does the application recover from the "can't deserialize" condition? does it keep re-caching the new object and continuing to not be able to deserialize it? or OK this is when you restart the application with a new version of the code is that it? we might want to add a rationale here is what I'm getting at.

Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mike bayer (zzzeek) wrote:

some doc tweaks.

also, add a changelog file as follows:

docs/build/unreleased/236.rst

.. change::
    :tags: feature, region
    :tickets: 236

    Added new construct :class:`.api.CantDeserializeException` which can be
    raised by user-defined deserializer functions which would be passed to
    :paramref:`.CacheRegion.deserializer`, to indicate a cache value that can't
    be deserialized and therefore should be regenerated. This can allow an
    application that's been updated to gracefully re-cache old items that were
    persisted from a previous version of the application. Pull request courtesy
    Simon Hewitt.

View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/dogpile.cache/+/4569

dogpile/cache/region.py Show resolved Hide resolved
Deserializers can raise a :class:`.api.CantDeserializeException` if they
are unable to deserialize the value from the backend, indicating
deserialization failed and that caching should proceed to re-generate
a value.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mike bayer (zzzeek) wrote:

then maybe another line with rationale: "this can allow an application that's been updated to gracefully re-cache old items that were persisted from a previous version of the application"

since I had to figure out why we want this

View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/dogpile.cache/+/4569

@sjhewitt
Copy link
Contributor Author

just to double check, the use case here is the unpickler would raise previously and cause a failure, here we have that it tells the get routine "NO VALUE" so it looks like the value is not in the cache, is that right?

Yep

how does the application recover from the "can't deserialize" condition? does it keep re-caching the new object and continuing to not be able to deserialize it? or OK this is when you restart the application with a new version of the code is that it? we might want to add a rationale here is what I'm getting at.

The internal issue I was solving is that we're using pickle to serialize/deserialize the values and the application code had been changed to move some packages around, meaning that pickle would error as it couldn't find the classes referenced by the old code, causing the whole application to crash.
This change means the app can create and cache a new value, which will be able to be successfully deserialized.

Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision f2ec265 of this pull request into gerrit so we can run tests and reviews and stuff

@sqla-tester
Copy link
Collaborator

Patchset f2ec265 added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/dogpile.cache/+/4569

@sqla-tester
Copy link
Collaborator

Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/dogpile.cache/+/4569 has been merged. Congratulations! :)

@zzzeek
Copy link
Member

zzzeek commented Apr 26, 2023

thanks, this is merged and dogpile.cache 1.2.0 is released. thanks for sticking around!

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

Successfully merging this pull request may close these issues.

3 participants