Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Cache] Handle serialization failures for Memcached#23615

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nicolas-grekas merged 1 commit intosymfony:3.3fromnicolas-grekas:cache-ser
Jul 26, 2017

Conversation

@nicolas-grekas
Copy link
Member

@nicolas-grekasnicolas-grekas commentedJul 21, 2017
edited
Loading

QA
Branch?3.3
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets-
LicenseMIT
Doc PR-

Fixes two issues with serialization + memcached: with the memcached extension, the default serializer is automatically selected as igbinary when possible, native php otherwise. That creates obvious migration/portability issues (ie just installing igbinary wipes out the value of your cache.)

Then, handling unserializing failures (esp. "php_incomplete_class") is a paramount feature of the component. You must be able to deal with migrating you code base without being blocked by some legacy serialized data.

@nicolas-grekas
Copy link
MemberAuthor

So, what would be your stand on#19895?

@robfrawley
Copy link
Contributor

robfrawley commentedJul 21, 2017
edited
Loading

Accidentally hit delete when editing my original message; here it is again:

Does this make the php serializer default now?! Do we really need to force this default for users, especially to an inferior serialization method? If people want portability and the environment's they deploy to differ, they can already explicitly set whatever serializer they want, right?

I think my initial reaction was an uneducated gut-reaction without fully understanding the intentions of this PR; if your goal is#19895 (comment), then I'm onboard, and may even have some time to contribute towards those goals.

@nicolas-grekas
Copy link
MemberAuthor

My goal with this PR is to have a predictable, thus safe, default behavior. :)

robfrawley reacted with heart emoji

Copy link
Member

@fabpotfabpot left a comment

Choose a reason for hiding this comment

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

I would merge this as a bug fix in 3.3 instead. Wiping the cache should never be an issue (should probably be done anyway on a new deploy).

@nicolas-grekasnicolas-grekas changed the base branch frommaster to3.3July 22, 2017 13:19
@nicolas-grekasnicolas-grekas modified the milestones:3.3,3.4Jul 22, 2017
@nicolas-grekas
Copy link
MemberAuthor

Rebased to target 3.3

@nicolas-grekasnicolas-grekas merged commitcccc88f intosymfony:3.3Jul 26, 2017
nicolas-grekas added a commit that referenced this pull requestJul 26, 2017
…as-grekas)This PR was merged into the 3.3 branch.Discussion----------[Cache] Handle serialization failures for Memcached| Q             | A| ------------- | ---| Branch?       | 3.3| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        | -Fixes two issues with serialization + memcached: with the memcached extension, the default serializer is automatically selected as igbinary when possible, native php otherwise. That creates obvious migration/portability issues (ie just installing igbinary wipes out the value of your cache.)Then, handling unserializing failures (esp. "php_incomplete_class") is a paramount feature of the component. You must be able to deal with migrating you code base without being blocked by some legacy serialized data.Commits-------cccc88f [Cache] Handle unserialization failures for Memcached
@nicolas-grekasnicolas-grekas deleted the cache-ser branchJuly 26, 2017 06:22
@fabpotfabpot mentioned this pull requestAug 1, 2017
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

3.3

Development

Successfully merging this pull request may close these issues.

4 participants

@nicolas-grekas@robfrawley@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp