[kudu-CR] [c++ client] added few deprecation notes
David Ribeiro Alves has posted comments on this change. Change subject: [c++ client] added few deprecation notes .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/4569/3//COMMIT_MSG Commit Message: Line 18: David Alves is anticipating some changes there: in short, the timestamp > Thanks for the reference -- I couldn't find that and decided to mention you not being inconsiderate, just that JIRA is more persistent than my memory :) -- To view, visit http://gerrit.cloudera.org:8080/4569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [c++ client] added few deprecation notes
Alexey Serbin has posted comments on this change. Change subject: [c++ client] added few deprecation notes .. Patch Set 3: (3 comments) Thank you for review! Will post updated version soon. http://gerrit.cloudera.org:8080/#/c/4569/3//COMMIT_MSG Commit Message: Line 18: David Alves is anticipating some changes there: in short, the timestamp > oh, also might be better to point to the JIRA (KUDU-611) that to me :) Thanks for the reference -- I couldn't find that and decided to mention your name instead. Will fix. Sorry for being inconsiderate here. http://gerrit.cloudera.org:8080/#/c/4569/3/src/kudu/client/client.h File src/kudu/client/client.h: Line 386: /// The latest observed timestamp can be used to start a snapshot scan on a > doesn't have to be in this patch, but it would be good to note that this co Good idea, will do. Actually, I'll just re-purpose this change list to cover both KUDU-1661 and KUDU-1663. PS3, Line 1816: one of > nit: s/one of/a Done -- To view, visit http://gerrit.cloudera.org:8080/4569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [c++ client] added few deprecation notes
David Ribeiro Alves has posted comments on this change. Change subject: [c++ client] added few deprecation notes .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/4569/3//COMMIT_MSG Commit Message: Line 18: David Alves is anticipating some changes there: in short, the timestamp oh, also might be better to point to the JIRA (KUDU-611) that to me :) -- To view, visit http://gerrit.cloudera.org:8080/4569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [c++ client] added few deprecation notes
David Ribeiro Alves has posted comments on this change. Change subject: [c++ client] added few deprecation notes .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/4569/3/src/kudu/client/client.h File src/kudu/client/client.h: Line 386: /// The latest observed timestamp can be used to start a snapshot scan on a doesn't have to be in this patch, but it would be good to note that this comment is a "workaround" for RYW and that, as you found out, the user needs to add 1 to actually get the correct timestamp back. PS3, Line 1816: one of nit: s/one of/a -- To view, visit http://gerrit.cloudera.org:8080/4569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [c++ client] added few deprecation notes
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4569 to look at the new patch set (#3). Change subject: [c++ client] added few deprecation notes .. [c++ client] added few deprecation notes KUDU-1661 Mark kudu::client::KuduClient::GetLatestObservedTimestamp() as experimental Added notes for methods related to getting and setting hybrid timestamps to perform READ_AT_SNAPSHOT scan: * KuduClient::GetLatestObservedTimestamp() * KuduClient::SetLatestObservedTimestamp() * KuduScanner::SetSnapshotRaw() David Alves is anticipating some changes there: in short, the timestamp might be replaced some opaque type (e.g., a sequence of bytes). Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 --- M src/kudu/client/client.h 1 file changed, 9 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/4569/3 -- To view, visit http://gerrit.cloudera.org:8080/4569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [c++ client] added few deprecation notes
David Ribeiro Alves has posted comments on this change. Change subject: [c++ client] added few deprecation notes .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4569/2/src/kudu/client/client.h File src/kudu/client/client.h: Line 390: /// @deprecated This method is experimental and will either disappear or > Unfortunately, there is no way to do that with current version of doxygen. I'd go for adding a @note and dropping the @deprecated tag, or just add "Note: This API is unstable." without any tags at all if you don't think there are any that make sense. -- To view, visit http://gerrit.cloudera.org:8080/4569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [c++ client] added few deprecation notes
Alexey Serbin has posted comments on this change. Change subject: [c++ client] added few deprecation notes .. Patch Set 2: (1 comment) > (1 comment) > > I think that the get/set latest timestamp APIs can be marked with > "unstable" or "experimental" or something, but I would delay actual > deprecation until there are replacements. Thank you for the feedback. I'll try to figure out whether it's possible to deliver that message via method/function attributes. As for now, I don't know how to do that. The only thing we can do in that regard is note-like doxygen attributes, so we can at least notify users of the API via the documentation. http://gerrit.cloudera.org:8080/#/c/4569/2/src/kudu/client/client.h File src/kudu/client/client.h: Line 390: /// @deprecated This method is experimental and will either disappear or > is there a way to mark it as "unstable" instead of deprecated? seems more a Unfortunately, there is no way to do that with current version of doxygen. Instead of @deprecated, we could add one of those notice-like tags: @note @warning @attention What do you think is the best one in the context? But of course, we need to remove the deprecated attribute from the method's signature. And I don't know what to put there instead to notify users of the API on the 'unstable' status of this API. -- To view, visit http://gerrit.cloudera.org:8080/4569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [c++ client] added few deprecation notes
David Ribeiro Alves has posted comments on this change. Change subject: [c++ client] added few deprecation notes .. Patch Set 2: (1 comment) I think that the get/set latest timestamp APIs can be marked with "unstable" or "experimental" or something, but I would delay actual deprecation until there are replacements. http://gerrit.cloudera.org:8080/#/c/4569/2/src/kudu/client/client.h File src/kudu/client/client.h: Line 390: /// @deprecated This method is experimental and will either disappear or is there a way to mark it as "unstable" instead of deprecated? seems more accurate -- To view, visit http://gerrit.cloudera.org:8080/4569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [c++ client] added few deprecation notes
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4569 to look at the new patch set (#2). Change subject: [c++ client] added few deprecation notes .. [c++ client] added few deprecation notes KUDU-1661 Mark kudu::client::KuduClient::GetLatestObservedTimestamp() as experimental Added deprecation notes for methods related to getting and setting hybrid timestamps to perform READ_AT_SNAPSHOT scan: * KuduClient::GetLatestObservedTimestamp() * KuduClient::SetLatestObservedTimestamp() * KuduScanner::SetSnapshotRaw() Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 --- M src/kudu/client/client.h 1 file changed, 18 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/4569/2 -- To view, visit http://gerrit.cloudera.org:8080/4569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [c++ client] added few deprecation notes
Alexey Serbin has posted comments on this change. Change subject: [c++ client] added few deprecation notes .. Patch Set 1: (1 comment) > (1 comment) > > Shouldn't we wait until the new API lands before deprecating the > old one though? That's a good question. The original intention for this change was to warn users of the API about the anticipated changes (as David mentioned in HipChat channel). We could just add a notice, warning or attention (@notice, @warning and @attention in Doxygen lingo) as comments, but I thought if were about to deprecate the methods pretty soon, I could just add the deprecation note along with corresponding clang/gcc attribute. It would be nice to collect more feedback on this. http://gerrit.cloudera.org:8080/#/c/4569/1/src/kudu/client/client.h File src/kudu/client/client.h: PS1, Line 396: its > Nit: "in signature". Or maybe just "or change in a future release" Done -- To view, visit http://gerrit.cloudera.org:8080/4569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [c++ client] added few deprecation notes
Adar Dembo has posted comments on this change. Change subject: [c++ client] added few deprecation notes .. Patch Set 1: (1 comment) Shouldn't we wait until the new API lands before deprecating the old one though? http://gerrit.cloudera.org:8080/#/c/4569/1/src/kudu/client/client.h File src/kudu/client/client.h: PS1, Line 396: its Nit: "in signature". Or maybe just "or change in a future release" Do the same in the other methods, and in the @deprecated tags. -- To view, visit http://gerrit.cloudera.org:8080/4569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [c++ client] added few deprecation notes
Alexey Serbin has uploaded a new change for review. http://gerrit.cloudera.org:8080/4569 Change subject: [c++ client] added few deprecation notes .. [c++ client] added few deprecation notes KUDU-1661 Mark kudu::client::KuduClient::GetLatestObservedTimestamp() as experimental Added deprecation notes for methods related to getting and setting hybrid timestamps to perform READ_AT_SNAPSHOT scan: * KuduClient::GetLatestObservedTimestamp() * KuduClient::SetLatestObservedTimestamp() * KuduScanner::SetSnapshotRaw() Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 --- M src/kudu/client/client.h 1 file changed, 18 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/4569/1 -- To view, visit http://gerrit.cloudera.org:8080/4569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6c45b797fa459ac9d214bb2612fd797f7a1eea45 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin
