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

Remove "failed to set key" warning from setMulti #490

Merged
merged 1 commit into from
Aug 23, 2021

Conversation

Krinkle
Copy link
Contributor

@Krinkle Krinkle commented Aug 18, 2021

This was introduced in 6837d89494, pull #214. I suspect it may have been a left-over from debugging something. The test was later changed in 6837d89 to expect the warning in question, although other similar tests don't encounter such warning currently.

It appears no other Memcached methods emit a PHP Warning when they encounter a write read or failure. Instead, they typically turn their return value into boolean false, and provide details via getResultMessage().

The introduction of this warning since php-memcached 3.0 has led to a number of confused consumers (locally #260, #409, #450, and more reports within downstream issue trackers).

Closes #409.

This was introduced in php-memcached-dev@6837d89494,
pull php-memcached-dev#214. I suspect it
may have been a left-over from debugging something. The test was later
changed in 6837d89 to expect the warning in question, although other
similar tests don't encounter such warning currently.

It appears no other Memcached methods emit a PHP Warning when they encounter
a write read or failure. Instead, they typically turn their return value
into boolean false, and provide details via getResultMessage().

The introduction of this warning since php-memcached 3.0 has led to a number
of confused consumers (locally php-memcached-dev#260, php-memcached-dev#409, php-memcached-dev#450, and more reports
within downstream issue trackers).

Closes php-memcached-dev#409.
@sodabrew sodabrew merged commit 6d45715 into php-memcached-dev:master Aug 23, 2021
@sodabrew
Copy link
Contributor

Well explained, thank you!

@sodabrew sodabrew added this to the 3.2.0 milestone Aug 23, 2021
@Krinkle Krinkle deleted the rm-setMulti-warn branch August 23, 2021 05:00
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.

setMulti failed to set key, set works normal?
2 participants