Fixed issue where add() function returns True when key already exists even when noreply is not set True

[OscarVanL]

This addresses the variation between the expected behaviour of the add() function in the documentation and its actual behaviour as described in #253.

The add() function should return True if a key is added successfully or False if a key cannot be added as the same key already exists in Memcached. This is, unless, the user sets noreply=True, where the function will always return True.

However, the actual behaviour is that it will always return True unless the user explicitly sets noreply=False, which is not how the documentation implies it works.

[OscarVanL]

It seems I had a misunderstanding in how noreply is setup to work in your implementation of Memcached. I've closed my issue/PR because nothing is wrong with your library, however, I do think it's odd that noreply=True by default. When I first read the documentation I assumed that noreply would just be an override if I wanted to disable replies, not something I'd explicitly need to set false for it to work in the way I desired.

[jparise]

@OscarVanL do you think we could improve this so others don't run into the same thing, perhaps through documentation?

[OscarVanL]

In my opinion, it shouldn't have noreply=True by default, it's inconsistent with other examples on the wiki.

I came from this example for 'ghetto central locking' using add() on the memcached wiki. I had expected add() in the python library to work consistently with this example without needing to add an extra noreply=False argument.

The documentation is wrong, in pymemcache/client/base.py, the docs for the function add() say:

    The memcached "add" command.
    Args:
      key: str, see class docs for details.
      value: str, see class docs for details.
      expire: optional int, number of seconds until the item is expired
              from the cache, or zero for no expiry (the default).
      noreply: optional bool, True to not wait for the reply (defaults to
               self.default_noreply).
      flags: optional int, arbitrary bit field used for server-specific
              flags
    Returns:
      **If noreply is True, the return value is always True. Otherwise the
      return value is True if the value was stored, and False if it was
      not (because the key already existed).**

The second sentence of 'Returns' is wrong, because if you don't set noreply to True (and omit the noreply argument), it will still default to always return True. That's where my confusion lied :)

[cgordon]

I went back and spent some time looking at the history. One of the earliest commits from me defaulted "noreply" to "True" in each method:

9cef80d

It looks like we added the "default_noreply" in 2015:

ca9d579

We have fixed a few minor bugs and documentation issues with "noreply" this year, but otherwise it has been relatively stable for the last four years.

I agree that "noreply" can be confusing, and I suspect that it would have been wiser to default it to "False", but it looks too late to go back now. The first sentence of "Returns" would be more accurate as: "If noreply is True (or if it is unset, and self.default_noreply is True), the return value is always True".

[jparise]

@cgordon, now that we've started 3.x major version development, we have an opportunity to change this default value. It doesn't sound like this would be a very friendly change to make for existing users, though.

[cgordon]

@jparise I thought about that, but I agree that it's a pretty unfriendly change. I think this is just part of the historic baggage of pymemcache now :)