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

After updated libcouchbase cberl:append not work #59

Closed
cclam0827 opened this issue Dec 16, 2014 · 12 comments
Closed

After updated libcouchbase cberl:append not work #59

cclam0827 opened this issue Dec 16, 2014 · 12 comments

Comments

@cclam0827
Copy link

After update libcouchbase from 2.4.1 to 2.4.4, I found that append function always return unknow error. Is somebody also have this issue?

In CentOS :
cberl:append(bucket,0, <<"test">>, <<"test">>).
{error,{unknown_error,58}}

In OSX:
cberl:append(bucket,0, <<"test">>, <<"test">>).
{error,unknown_error}

@wcummings
Copy link
Contributor

58 is the detailed error code

  /**An option was passed to a command which is incompatible with other
     * options. This may happen if two fields are mutually exclusive */ \
    X(LCB_OPTIONS_CONFLICT, 0x3A, LCB_ERRTYPE_INPUT, \
      "The operation structure contains conflicting options")

I'll look into this

@wcummings
Copy link
Contributor

Looks like the newer version is stricter, flags should not be set for append/prepend:

    case LCB_APPEND:
    case LCB_PREPEND:
        if (cmd->exptime || cmd->flags) {
            return LCB_OPTIONS_CONFLICT;
        }
        break;

@cclam0827
Copy link
Author

Can we keep the api append/4?
like this

append(PoolPid, Cas, Key, Value) ->
   store(PoolPid, append, Key, Value, none, 0, 0).

or

append(PoolPid, Cas, Key, Value) ->
    append(PoolPid, Key, Value).
append(PoolPid, Key, Value) ->
    store(PoolPid, append, Key, Value, none, 0, 0).

@sjmackenzie
Copy link

Typically is bad form to breaking public APIs. Unless of course there is an
overriding consensus.

@wcummings
Copy link
Contributor

It's easy enough to avoid in this case, but it will happen from time to time. I will add deprecated functions for append and prepend which accept an unused CAS value

@sjmackenzie
Copy link

Understood, rock and a hard place, never easy, though this project is
starting to become more mature with production code using it as a
dependency so public API stability is rather important indeed.

I must thank you for your amazing contribution, most appreciated :)

@hlieberman
Copy link
Contributor

@sjmackenzie, it's my fault. I normally try to keep us pretty close to SemVer, and I didn't look closely at the patch set to see that there was a breaking change in there. We're not always going to be able to avoid them, but I'll try to make sure that they're not a surprise, at least.

@vincentdephily
Copy link
Contributor

With that and returning {ok,CAS} and having a flexible options argument for storage functions, it would seem like a good idea to bundle as many pending API changes as desired into a 1.1 branch, and keep non-api-breaking patches for master in the meantime ? You can then decide how long the 1.0 branch should be supported.

@sjmackenzie
Copy link

my personal preference is to use the zeromq development 'contract'
inthat there is only the master branch on the canonical repository and
stable APIs dont change.APIs have a lifecycle, being annotated with
'experimental', 'stable', 'depricated' and 'legacy'. Having a
predictable stable API is a softwares greatest assert. Im terribly
sorry that you're building on couchbase' quicksand foundation and i'll
reuse the expression, are caught between a rock and a hard place. Have
a read of the ZeroMQ development methodology:
http://rfc.zeromq.org/spec:16

@sjmackenzie
Copy link

(forgive my mobile phone typos)

@wcummings
Copy link
Contributor

I've long considered creating a "v2" API, while maintaining this one. Now might be the right time to do that, with the current API being 'stable' and the new API being 'experimental'. The biggest issue is that the current API doesn't provide for many ways to extend it w/o breaking compatibility.

I have a changeset for the deprecated append/prepend functions, so I will merge those in shortly and we can hold off and put the CAS changes into a new experimental branch.

Is this agreeable? @sjmackenzie @vincentdephily @hlieberman

@sjmackenzie
Copy link

Yes its a good way albeit using a different name. Names, if used in
the context of C4, stay around forever :)

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

No branches or pull requests

5 participants