[kudu-CR] [sentry] add SentryAuthzProvider
Dan Burkert has posted comments on this change. (
http://gerrit.cloudera.org:8080/11659 )
Change subject: [sentry] add SentryAuthzProvider
..
Patch Set 4:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/11659/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11659/4//COMMIT_MSG@10
PS4, Line 10: retries, reconnections, HA, error
: handling, and concurrent requests
this is all handled by the existing thrift::HaClient.
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/authz_provider.h
File src/kudu/master/authz_provider.h:
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/authz_provider.h@67
PS4, Line 67: AuthorizeGetTableMetadata
Where is this intended to be used?
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider-test.cc
File src/kudu/master/sentry_authz_provider-test.cc:
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider-test.cc@67
PS4, Line 67: ="
whitespace here and below
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider.h
File src/kudu/master/sentry_authz_provider.h:
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider.h@57
PS4, Line 57: OVERRIDE
'override' here and below
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider.h@88
PS4, Line 88: virtual Status AuthorizeGetTableMetadata(const std::string&
table_name,
Can you remind me what master/catalog manager ops need to call this?
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider.h@111
PS4, Line 111: or 'kudu'
: // (Kudu server name) is a non admin user in Sentry
Is there any way we can recognize this failure scenario as part of Start()? I
think the earlier we can catch this, the easier it will be to surface a good
error message, and consequently debug.
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider.cc
File src/kudu/master/sentry_authz_provider.cc:
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider.cc@110
PS4, Line 110: // then the creating user must have 'ALL on DATABASE' with
grant.
Interesting - is there a link or somewhere you can reference in Sentry that
explains why this is?
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider.cc@140
PS4, Line 140: // For table alteration (with table rename) requires
Same thing here, I think there's a SENTRY JIRA that explains this distinction,
would be good to link for future reference.
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider.cc@187
PS4, Line 187: // Note: the RPC addresses of the Sentry service(s) can be
with/without
We already have HostPort::ParseStrings which does almost the exact same thing
as this, however it doesn't handle schemes. I'd like to see this use that
method instead of implementing largely the same logic again, which means either
we need to not handle addresses with schemes, or add scheme support to
ParseStrings. I'm leaning toward the former since I don't think schemes are
used much for Sentry, but I don't really have data to back that up. I'm
curious what others think.
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider.cc@244
PS4, Line 244: (privilege.grantOption == TSentryGrantOption::UNSET ||
: privilege.grantOption ==
TSentryGrantOption::FALSE)
might be a bit safer to express this as 'privilege.grantOption !=
TSentryGrantOption::TRUE'. It's a bit simpler, and in the unlikely case that
an additional variant is added it will 'fail closed'.
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider.cc@250
PS4, Line 250: Status s = SentryAction::FromString(privilege.action,
_action);
I'd add a WARN_NOT_OK or similar here so that we get an indication in the logs
if we aren't handling a new action type. This is especially useful since we
are returning a non-specific error.
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider.cc@256
PS4, Line 256: Substitute
substitute isn't necessary
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider.cc@256
PS4, Line 256: return Status::NotAuthorized(Substitute("unauthorized
action"));
Is there a security reason to have a generic error message? It may be helpful
to try and explain why the action is unauthorized, but maybe that could be a
sidechannel leak? I don't have anything specific in mind, but it seems like a
possible risk. In any case there should probably be a comment explaining which
way we go.
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider.cc@275
PS4, Line 275: CHECK(false) << "unsupported authorizable scope";
Prefer LOG(FATAL)
--
To
[kudu-CR] [sentry] add SentryAuthzProvider
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/11659 )
Change subject: [sentry] add SentryAuthzProvider
..
Patch Set 4:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/11659/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11659/4//COMMIT_MSG@7
PS4, Line 7: [sentry] add SentryAuthzProvider
Could you add a paragraph explaining the module placement choices?
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/authz_provider.h
File src/kudu/master/authz_provider.h:
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/authz_provider.h@59
PS4, Line 59: AuthorizeAlterTable
Should we wrap new_table in a boost::optional, to convey that it isn't
necessary for non-rename alters?
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider.h
File src/kudu/master/sentry_authz_provider.h:
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider.h@56
PS4, Line 56:
: virtual Status Start() OVERRIDE;
:
: virtual void Stop() OVERRIDE;
Nit: lower-case override for new code.
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider.cc
File src/kudu/master/sentry_authz_provider.cc:
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider.cc@262
PS4, Line 262: Slice database;
: Slice table;
: RETURN_NOT_OK(ParseHiveTableIdentifier(table_name, ,
));
I commented on this before, but this can be scoped to TABLE or DATABASE; it's
not necessary for SERVER.
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/master/sentry_authz_provider.cc@267
PS4, Line 267: case AuthorizableScope::TABLE:
: authorizable->__set_table(table.ToString());
: case AuthorizableScope::DATABASE:
: authorizable->__set_db(database.ToString());
Missing break statements here? Or if this is intentional, there's a
FALLTHROUGH_INTENDED macro.
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/sentry/sentry-test-base.h
File src/kudu/sentry/sentry-test-base.h:
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/sentry/sentry-test-base.h@33
PS4, Line 33: class SentryTestBase : public KuduTest {
Seems like you could push the Kerberos boolean parameterization down into this
fixture, as well as the SetUp() and TearDown() methods.
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/sentry/sentry-test-base.h@67
PS4, Line 67: protected:
Nit: indent by one character.
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/sentry/sentry_client-test.cc
File src/kudu/sentry/sentry_client-test.cc:
http://gerrit.cloudera.org:8080/#/c/11659/4/src/kudu/sentry/sentry_client-test.cc@60
PS4, Line 60: void SetUp() override {
Note that when overriding SetUp()/TearDown() you should make sure to chain to
the superclass implementations. For SetUp() it should be the first thing you
do; for TearDown(), the last.
--
To view, visit http://gerrit.cloudera.org:8080/11659
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I254828d640cd905e33dbaf0fe100d660bc9e6772
Gerrit-Change-Number: 11659
Gerrit-PatchSet: 4
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 26 Oct 2018 00:19:50 +
Gerrit-HasComments: Yes
[kudu-CR](branch-1.8.x) [doc] run doxygen with no warnings
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11792 ) Change subject: [doc] run doxygen with no warnings .. [doc] run doxygen with no warnings Updated doxygen configuration due to recent changes in Kudu C++ client sample application. Cleaned up other minor documentation issues in src/kudu/client/schema.h. With this patch, doxygen runs without any warnings while auto-generating documentation for Kudu C++ client API (verified with doxygen 1.8.14 built on OS X 10.11.6). This is a follow-up for a30c9211e8e73ead70acdb6108e5a0eac90cb5cb. Change-Id: Iedb31849288caa59456aff6eb075cf23c843d4e0 Reviewed-on: http://gerrit.cloudera.org:8080/11784 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo (cherry picked from commit 9042eda2c6c055c364f9c441c9b7fab3ecd15649) Reviewed-on: http://gerrit.cloudera.org:8080/11792 Reviewed-by: Attila Bukor --- M CMakeLists.txt M docs/support/doxygen/client_api.doxy.in M src/kudu/client/schema.h 3 files changed, 10 insertions(+), 1 deletion(-) Approvals: Kudu Jenkins: Verified Attila Bukor: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11792 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.8.x Gerrit-MessageType: merged Gerrit-Change-Id: Iedb31849288caa59456aff6eb075cf23c843d4e0 Gerrit-Change-Number: 11792 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [sentry] add SentryAuthzProvider
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/11659 )
Change subject: [sentry] add SentryAuthzProvider
..
Patch Set 3:
(38 comments)
http://gerrit.cloudera.org:8080/#/c/11659/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11659/3//COMMIT_MSG@7
PS3, Line 7: [sentry] SentryAuthzProvider
> Nit: this is a little sparse; maybe convert into a sentence (or sentence fr
Done
http://gerrit.cloudera.org:8080/#/c/11659/3//COMMIT_MSG@9
PS3, Line 9: the Sentry
> Nit: "above Sentry"
Done
http://gerrit.cloudera.org:8080/#/c/11659/3//COMMIT_MSG@13
PS3, Line 13: SentryAuthzPrivider
> SentryAuthzProvider
Done
http://gerrit.cloudera.org:8080/#/c/11659/3/src/kudu/sentry/sentry_authz_provider-test.cc
File src/kudu/sentry/sentry_authz_provider-test.cc:
http://gerrit.cloudera.org:8080/#/c/11659/3/src/kudu/sentry/sentry_authz_provider-test.cc@72
PS3, Line 72: if (enable_kerberos) {
> I think I've seen this block of code at least 3 times now. Can we consolida
Done
http://gerrit.cloudera.org:8080/#/c/11659/3/src/kudu/sentry/sentry_authz_provider-test.cc@72
PS3, Line 72: if (enable_kerberos) {
> +1, pull this out into some Sentry test base and use it here and in sentry_
Done
http://gerrit.cloudera.org:8080/#/c/11659/3/src/kudu/sentry/sentry_authz_provider-test.cc@109
PS3, Line 109: ASSERT_OK(sentry_client_->Stop());
> Shouldn't we stop the client before the server?
Done
http://gerrit.cloudera.org:8080/#/c/11659/3/src/kudu/sentry/sentry_authz_provider-test.cc@121
PS3, Line 121: SentryAuthzProviderTest
> Is it worth adding a few scenarios to verify how the provider works if Sent
Done
http://gerrit.cloudera.org:8080/#/c/11659/3/src/kudu/sentry/sentry_authz_provider-test.cc@127
PS3, Line 127: ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString();
> Why is this an error at all? Why not just set `authorized` to false? How do
This is an error returned from Sentry service. I think these errors should
return it back to users.
http://gerrit.cloudera.org:8080/#/c/11659/3/src/kudu/sentry/sentry_authz_provider-test.cc@127
PS3, Line 127: ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString();
> This is surprising; shouldn't it be NotFound() or something like that?
Yeah, it is strange Sentry is returning as AccessDenied error. I filed a jira
https://issues.apache.org/jira/browse/SENTRY-2434 to track this.
http://gerrit.cloudera.org:8080/#/c/11659/3/src/kudu/sentry/sentry_authz_provider-test.cc@135
PS3, Line 135: // Authorize create table on a user without required privileges.
:
> nit: extra line
The extra line is to indicate the comment is for the following blocks.
http://gerrit.cloudera.org:8080/#/c/11659/3/src/kudu/sentry/sentry_authz_provider-test.cc@146
PS3, Line 146: group_requset
> group_request
Done
http://gerrit.cloudera.org:8080/#/c/11659/3/src/kudu/sentry/sentry_authz_provider-test.cc@137
PS3, Line 137: TCreateSentryRoleRequest role_req;
: role_req.__set_requestorUserName("test-admin");
: role_req.__set_roleName("developer");
: ASSERT_OK(sentry_client_->CreateRole(role_req));
:
: TSentryGroup group;
: group.groupName = "user";
: set groups;
: groups.insert(group);
: TAlterSentryRoleAddGroupsRequest group_requset;
: TAlterSentryRoleAddGroupsResponse group_response;
: group_requset.__set_requestorUserName("test-admin");
: group_requset.__set_roleName("developer");
: group_requset.__set_groups(groups);
: ASSERT_OK(sentry_client_->AlterRoleAddGroups(group_requset,
_response));
> This is copied in every test, so maybe pull it into its own function (or fu
Done
http://gerrit.cloudera.org:8080/#/c/11659/3/src/kudu/sentry/sentry_authz_provider-test.cc@204
PS3, Line 204: TSentryPrivilege privilege;
: privilege.__set_privilegeScope("DATABASE");
: privilege.__set_serverName(FLAGS_server_name);
: privilege.__set_dbName("db");
: privilege.__set_action("SELECT");
> nit: We're using these _a lot_ so maybe it's worth introducing PrivilegeFor
Done
http://gerrit.cloudera.org:8080/#/c/11659/3/src/kudu/sentry/sentry_authz_provider.h
File src/kudu/sentry/sentry_authz_provider.h:
http://gerrit.cloudera.org:8080/#/c/11659/3/src/kudu/sentry/sentry_authz_provider.h@41
PS3, Line 41: the Sentry
> just Sentry, not "the Sentry"
Done
http://gerrit.cloudera.org:8080/#/c/11659/3/src/kudu/sentry/sentry_authz_provider.h@60
PS3, Line 60: // Check if the table creation operation on a given table is
authorized
> Nit: Start/Stop use present tense, so the rest of these methods should too.
Done
http://gerrit.cloudera.org:8080/#/c/11659/3/src/kudu/sentry/sentry_authz_provider.h@61
PS3, Line 61: Return the authorization result in out
[kudu-CR] [sentry] add SentryAuthzProvider
Hello Tidy Bot, Alexey Serbin, Dan Burkert, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11659 to look at the new patch set (#4). Change subject: [sentry] add SentryAuthzProvider .. [sentry] add SentryAuthzProvider This commit adds a higher-level API above Sentry which handles authorizations on Kudu operations, retries, reconnections, HA, error handling, and concurrent requests. A follow-up commit will integrate the SentryAuthzProvider into the CatalogManager to perform authorization checks on Master RPCs. Change-Id: I254828d640cd905e33dbaf0fe100d660bc9e6772 --- M src/kudu/master/CMakeLists.txt A src/kudu/master/authz_provider.h A src/kudu/master/default_authz_provider.h A src/kudu/master/sentry_authz_provider-test.cc A src/kudu/master/sentry_authz_provider.cc A src/kudu/master/sentry_authz_provider.h A src/kudu/sentry/sentry-test-base.h M src/kudu/sentry/sentry_action-test.cc M src/kudu/sentry/sentry_action.cc M src/kudu/sentry/sentry_action.h M src/kudu/sentry/sentry_client-test.cc M src/kudu/sentry/sentry_client.cc M src/kudu/sentry/sentry_client.h 13 files changed, 994 insertions(+), 83 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/11659/4 -- To view, visit http://gerrit.cloudera.org:8080/11659 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I254828d640cd905e33dbaf0fe100d660bc9e6772 Gerrit-Change-Number: 11659 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR](gh-pages) Updating web site for Kudu 1.8.0 release
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11772 ) Change subject: Updating web site for Kudu 1.8.0 release .. Patch Set 4: > Patch Set 4: > > > > Patch Set 4: Code-Review+1 > > > > > > > > Patch Set 4: > > > > > > > > > > (1 comment) > > > > > > > > Go be honest, I'm not sure if it's a good idea to do that. I > > mean > > > > if someone builds the documentation on their own, it's going > > to be > > > > different from what's on the website and it's too late to cut > > a new > > > > RC as I archived 1.8.0 yesterday. > > > > > > > > On the other hand, I can see why it would be good as most devs > > > > won't use their own doxygen build if it's publicly available. > > > > > > > > What does everyone think? I'm -0 on this. > > > > > > > > Anyway, Status ListPartitions() is not part of 1.8.0 and > > backported > > > > a30c921 on top of branch-1.8.x (https://gerrit.cloudera.org/c/11783) > > > > so I can rebuild the docs if needed or at least it can be > > fixed in > > > > 1.8.1. > > > > > > Sorry for the trouble. I don't feel strong about the need to > > rebuild the docs for 1.8.0: if fixing that in 1.8.1 is the easiest > > way to go, that sounds good to me. > > > > I'm not really saying it's the easiest, as rebuilding the docs is > > straight-forward enough, but I feel it's cleaner if the docs are > > the same on the site and in the released sources. > > Right, I agree -- keeping things consistent is a noble goal. So, having > those docs updated in 1.8.1 looks a better deal to me as well. Thanks! I talked with Todd on Slack, he said it should be okay to publish newer docs than in the tarball if it doesn't make the docs inaccurate. I might be overcomplicating, but I was thinking maybe we could have this checked in this way, then rm the cpp-client-api symlink and create an actual directory with the fixed docs. That way the website would show the fixed docs, but the /release/1.8.0/cpp-client-api docs would still be the actual 1.8.0 docs. What do you think? -- To view, visit http://gerrit.cloudera.org:8080/11772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: Iafad2ae1cc4ae5700025ea3167d51c6f886a2240 Gerrit-Change-Number: 11772 Gerrit-PatchSet: 4 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anonymous Coward (314) Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: helifu Gerrit-Comment-Date: Thu, 25 Oct 2018 23:43:47 + Gerrit-HasComments: No
[kudu-CR] docs: Add simplest possible Spark SQL example
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11095 ) Change subject: docs: Add simplest possible Spark SQL example .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/11095/2/docs/developing.adoc File docs/developing.adoc: http://gerrit.cloudera.org:8080/#/c/11095/2/docs/developing.adoc@109 PS2, Line 109: kudu spark nit: kudu-spark http://gerrit.cloudera.org:8080/#/c/11095/2/docs/developing.adoc@110 PS2, Line 110: dataframe nit: same here http://gerrit.cloudera.org:8080/#/c/11095/2/docs/developing.adoc@110 PS2, Line 110: dataframe nit: DataFrame -- To view, visit http://gerrit.cloudera.org:8080/11095 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2cf4c00f3a1dc92fd93458aa3c1b1d2cd4f38f78 Gerrit-Change-Number: 11095 Gerrit-PatchSet: 2 Gerrit-Owner: Mike Percy Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Thu, 25 Oct 2018 23:23:32 + Gerrit-HasComments: Yes
[kudu-CR](branch-1.8.x) [doc] run doxygen with no warnings
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11792 ) Change subject: [doc] run doxygen with no warnings .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11792 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.8.x Gerrit-MessageType: comment Gerrit-Change-Id: Iedb31849288caa59456aff6eb075cf23c843d4e0 Gerrit-Change-Number: 11792 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 25 Oct 2018 23:17:33 + Gerrit-HasComments: No
[kudu-CR] [examples] Add basic Spark example written in Scala
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11788 ) Change subject: [examples] Add basic Spark example written in Scala .. Patch Set 4: Verified+1 unrelated TSAN failure -- To view, visit http://gerrit.cloudera.org:8080/11788 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f Gerrit-Change-Number: 11788 Gerrit-PatchSet: 4 Gerrit-Owner: Mitch Barnett Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mitch Barnett Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 25 Oct 2018 23:16:47 + Gerrit-HasComments: No
[kudu-CR] [examples] Add basic Spark example written in Scala
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11788 ) Change subject: [examples] Add basic Spark example written in Scala .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11788 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f Gerrit-Change-Number: 11788 Gerrit-PatchSet: 4 Gerrit-Owner: Mitch Barnett Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mitch Barnett Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 25 Oct 2018 23:16:36 + Gerrit-HasComments: No
[kudu-CR] [examples] Add basic Spark example written in Scala
Adar Dembo has removed a vote on this change. Change subject: [examples] Add basic Spark example written in Scala .. Removed Code-Review+1 by Mitch Barnett -- To view, visit http://gerrit.cloudera.org:8080/11788 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f Gerrit-Change-Number: 11788 Gerrit-PatchSet: 4 Gerrit-Owner: Mitch Barnett Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mitch Barnett Gerrit-Reviewer: Will Berkeley
[kudu-CR] [examples] Add basic Spark example written in Scala
Attila Bukor has removed a vote on this change. Change subject: [examples] Add basic Spark example written in Scala .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/11788 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f Gerrit-Change-Number: 11788 Gerrit-PatchSet: 4 Gerrit-Owner: Mitch Barnett Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mitch Barnett Gerrit-Reviewer: Will Berkeley
[kudu-CR] [examples] Add basic Spark example written in Scala
Mitch Barnett has posted comments on this change. (
http://gerrit.cloudera.org:8080/11788 )
Change subject: [examples] Add basic Spark example written in Scala
..
Patch Set 4: Code-Review+1
(11 comments)
Updated per Grant's feedback.
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/README.adoc
File examples/scala/spark-example/README.adoc:
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/README.adoc@18
PS1, Line 18: = Kudu-Spark example README
> nit: extra space
Done
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/README.adoc@40
PS1, Line 40: - KuduMasters: A String value consisting of a comma-separated
list of Kudu Master Host addresses. This will need to be pointed to the Kudu
cluster you wish to target.
> Is this needed? Given the code is creating the table, we have control over
Good call - probably better to give less control as to not cause additional
failures.
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/README.adoc@43
PS1, Line 43: To specify a value at execution time, you'll specify the
parameter name in the '-D' format. For example, to set a different set of
masters for the Kudu cluster from the command line and use a custom table name,
set the property `KuduMasters` to a CSV of the master addresses in the form
`host:port` and add a table name value, as shown:
> nit: Move up near KUDU_MASTERS
I didn't see any default listed for this value - it's inclusion is only due to
it being a requirement of the SparkSession create statement.
I suppose you could infer that 'local' is the default value, since any other
value would require a spark master to be explicitly defined.
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala
File
examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala:
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@14
PS1, Line 14: object SparkExample {
> nit: val KuduMasters: String = ...
Done
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@19
PS1, Line 19: val NameCol = "name"
> Should this be configurable like KUDU_MASTERS?
I went ahead and made it configurable, as to allow those who want to the option
of changing where it runs.
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@32
PS1, Line 32:
> Nit: I think the below sytax is easier
Went ahead and made that syntax change - bit easier to read.
Also went ahead and explicitly defined 'false' for the structFields instead of
the NULLABLE constant.
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@39
PS1, Line 39:
> nit: Replication factor of 1 might be easier for examples.
Done
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@43
PS1, Line 43: )
> Can this be moved up by the other imports.
From what I've read in the Spark documentation and what I've seen in the code
base, it can't be moved.
It's being called directly from the SparkSession we instantiated above.
I could move it higher in main(), but I can't take it up to the other imports
unfortunately.
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@47
PS1, Line 47: if (!kc.tableExists(TableName)) {
> Nit: Use slf4j logging.
Done
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@77
PS1, Line 77: //create a DF and map the values of the KuduMaster and
TableName values
> No need to catch if there is no handling. Just let the exception bubble up.
Done
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@87
PS1, Line 87: kc.deleteTable(TableName)
> No need to catch here either.
Done
--
To view, visit http://gerrit.cloudera.org:8080/11788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f
Gerrit-Change-Number: 11788
Gerrit-PatchSet: 4
Gerrit-Owner: Mitch Barnett
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mitch Barnett
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Thu, 25 Oct 2018 23:11:25 +
Gerrit-HasComments: Yes
[kudu-CR] [examples] Add basic Spark example written in Scala
Hello Will Berkeley, Attila Bukor, Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11788 to look at the new patch set (#4). Change subject: [examples] Add basic Spark example written in Scala .. [examples] Add basic Spark example written in Scala This patch adds a basic Kudu client that utilizes both Kudu Java APIs, as well as Spark SQL APIs. It will allow customers to pull down the pom.xml and scala source, then build and execute from their local machine. Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f --- A examples/scala/spark-example/README.adoc A examples/scala/spark-example/pom.xml A examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala 3 files changed, 259 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/11788/4 -- To view, visit http://gerrit.cloudera.org:8080/11788 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f Gerrit-Change-Number: 11788 Gerrit-PatchSet: 4 Gerrit-Owner: Mitch Barnett Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley
[kudu-CR] [examples] Add basic Spark example written in Scala
Hello Will Berkeley, Attila Bukor, Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11788 to look at the new patch set (#3). Change subject: [examples] Add basic Spark example written in Scala .. [examples] Add basic Spark example written in Scala This patch adds a basic Kudu client that utilizes both Kudu Java APIs, as well as Spark SQL APIs. It will allow customers to pull down the pom.xml and scala source, then build and execute from their local machine. Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f --- A examples/scala/spark-example/README.adoc A examples/scala/spark-example/pom.xml A examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala 3 files changed, 257 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/11788/3 -- To view, visit http://gerrit.cloudera.org:8080/11788 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f Gerrit-Change-Number: 11788 Gerrit-PatchSet: 3 Gerrit-Owner: Mitch Barnett Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley
[kudu-CR] [examples] Add basic Spark example written in Scala
Hello Will Berkeley, Attila Bukor, Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11788 to look at the new patch set (#2). Change subject: [examples] Add basic Spark example written in Scala .. [examples] Add basic Spark example written in Scala This patch adds a basic Kudu client that utilizes both Kudu Java APIs, as well as Spark SQL APIs. It will allow customers to pull down the pom.xml and scala source, then build and execute from their local machine. Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f --- A examples/scala/spark-example/README.adoc A examples/scala/spark-example/pom.xml A examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala 3 files changed, 259 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/11788/2 -- To view, visit http://gerrit.cloudera.org:8080/11788 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f Gerrit-Change-Number: 11788 Gerrit-PatchSet: 2 Gerrit-Owner: Mitch Barnett Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley
[kudu-CR](gh-pages) [blog] Add post about 1.8.0 release
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11776 ) Change subject: [blog] Add post about 1.8.0 release .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11776 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: I99fd23683e7417ca9bc836edf49a7e02e36c6b74 Gerrit-Change-Number: 11776 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 25 Oct 2018 21:50:10 + Gerrit-HasComments: No
[kudu-CR] [client] expose location via internal API
Fengling Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/11679 ) Change subject: [client] expose location via internal API .. Patch Set 12: Alexey, thanks for rebasing! -- To view, visit http://gerrit.cloudera.org:8080/11679 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib92cc6806073d32c859ae44ff803abb37cac99ac Gerrit-Change-Number: 11679 Gerrit-PatchSet: 12 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 25 Oct 2018 21:33:10 + Gerrit-HasComments: No
[kudu-CR](branch-1.8.x) [doc] run doxygen with no warnings
Hello Kudu Jenkins, Adar Dembo, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/11792 to review the following change. Change subject: [doc] run doxygen with no warnings .. [doc] run doxygen with no warnings Updated doxygen configuration due to recent changes in Kudu C++ client sample application. Cleaned up other minor documentation issues in src/kudu/client/schema.h. With this patch, doxygen runs without any warnings while auto-generating documentation for Kudu C++ client API (verified with doxygen 1.8.14 built on OS X 10.11.6). This is a follow-up for a30c9211e8e73ead70acdb6108e5a0eac90cb5cb. Change-Id: Iedb31849288caa59456aff6eb075cf23c843d4e0 Reviewed-on: http://gerrit.cloudera.org:8080/11784 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo (cherry picked from commit 9042eda2c6c055c364f9c441c9b7fab3ecd15649) --- M CMakeLists.txt M docs/support/doxygen/client_api.doxy.in M src/kudu/client/schema.h 3 files changed, 10 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/11792/1 -- To view, visit http://gerrit.cloudera.org:8080/11792 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.8.x Gerrit-MessageType: newchange Gerrit-Change-Id: Iedb31849288caa59456aff6eb075cf23c843d4e0 Gerrit-Change-Number: 11792 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [client] expose location via internal API
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11679 ) Change subject: [client] expose location via internal API .. [client] expose location via internal API I added location info to client::KuduTabletServer and client::internal::RemoteTabletServer to enable the future work on location-aware C++ and Java clients. Change-Id: Ib92cc6806073d32c859ae44ff803abb37cac99ac Reviewed-on: http://gerrit.cloudera.org:8080/11679 Reviewed-by: Adar Dembo Tested-by: Alexey Serbin --- M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/scan_token-internal.cc M src/kudu/client/tablet_server-internal.cc M src/kudu/client/tablet_server-internal.h 7 files changed, 34 insertions(+), 7 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Alexey Serbin: Verified -- To view, visit http://gerrit.cloudera.org:8080/11679 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ib92cc6806073d32c859ae44ff803abb37cac99ac Gerrit-Change-Number: 11679 Gerrit-PatchSet: 12 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Will Berkeley
[kudu-CR] [client] expose location via internal API
Alexey Serbin has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/11679 ) Change subject: [client] expose location via internal API .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/11679 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Ib92cc6806073d32c859ae44ff803abb37cac99ac Gerrit-Change-Number: 11679 Gerrit-PatchSet: 11 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Will Berkeley
[kudu-CR] [examples] Add basic Spark example written in Scala
Grant Henke has posted comments on this change. (
http://gerrit.cloudera.org:8080/11788 )
Change subject: [examples] Add basic Spark example written in Scala
..
Patch Set 1:
(11 comments)
Thanks for the contribution. I did a quick first review pass.
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/README.adoc
File examples/scala/spark-example/README.adoc:
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/README.adoc@18
PS1, Line 18: = Kudu-Spark example README
nit: extra space
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/README.adoc@40
PS1, Line 40: - ID_COL: String value containing the name of the Primary Key
column.
Is this needed? Given the code is creating the table, we have control over
this. Same with NAME_COL. I think it complicates the example.
Looking at the code, these aren't exposed as setable properties.
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/README.adoc@43
PS1, Line 43: - SPARK_MASTER: String value which identifies the location of the
Spark Master to be used. If running locally (standalone), this must be set to
'local'.
nit: Move up near KUDU_MASTERS
Can you specify if these have a default?
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala
File
examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala:
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@14
PS1, Line 14: val KUDU_MASTERS : String =
System.getProperty("KUDU_MASTERS","kudu.master1:7051,kudu.master2:7051,kudu.master3:7051")
//kudu master address list
nit: val KuduMasters: String = ...
In Scala constants are usually named like objects. The others below should be
changed too.
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@19
PS1, Line 19: val SPARK_MASTER = "local" //location of spark master, specify
'local' if running on localhost
Should this be configurable like KUDU_MASTERS?
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@32
PS1, Line 32: val schema : StructType = StructType {
Nit: I think the below sytax is easier
StructType(
List(
StructField("ID_COL", DataTypes.IntegerType,NULLABLE),
StructField("NAME_COL", DataTypes.StringType,NULLABLE),
))
ID columns can't be nullable. You can probably just hard coded if a column is
nullable or not.
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@39
PS1, Line 39: ID_COL
nit: Replication factor of 1 might be easier for examples.
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@43
PS1, Line 43: import spark.implicits._
Can this be moved up by the other imports.
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@47
PS1, Line 47: System.out.println("Writing to table " + TABLE_NAME)
Nit: Use slf4j logging.
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@77
PS1, Line 77: case e: Exception => e.printStackTrace()
No need to catch if there is no handling. Just let the exception bubble up.
http://gerrit.cloudera.org:8080/#/c/11788/1/examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala@87
PS1, Line 87: case e: Exception => e.printStackTrace()
No need to catch here either.
--
To view, visit http://gerrit.cloudera.org:8080/11788
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f
Gerrit-Change-Number: 11788
Gerrit-PatchSet: 1
Gerrit-Owner: Mitch Barnett
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Will Berkeley
Gerrit-Comment-Date: Thu, 25 Oct 2018 19:46:01 +
Gerrit-HasComments: Yes
[kudu-CR](branch-1.8.x) [c++ client] clean up inline doxygen documentation
Attila Bukor has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11783 ) Change subject: [c++ client] clean up inline doxygen documentation .. [c++ client] clean up inline doxygen documentation This patch cleans up doxygen documentation in client.h. Most importantly, this patch removes the internal API methods (i.e. methods marked with KUDU_NO_EXPORT attribute) from the auto-generated documentation. Fixed other typos and mistakes in doxygen formatting, so now doxygen outputs less warnings. The rest of the doxygen warnings will be addressed in a follow-up patch. Change-Id: Ib0038f45e746896b6abbcfaa5741760218d7a9ad Reviewed-on: http://gerrit.cloudera.org:8080/11780 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo (cherry picked from commit a30c9211e8e73ead70acdb6108e5a0eac90cb5cb) Reviewed-on: http://gerrit.cloudera.org:8080/11783 Reviewed-by: Alexey Serbin --- M src/kudu/client/client.h 1 file changed, 53 insertions(+), 21 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11783 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.8.x Gerrit-MessageType: merged Gerrit-Change-Id: Ib0038f45e746896b6abbcfaa5741760218d7a9ad Gerrit-Change-Number: 11783 Gerrit-PatchSet: 3 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward (314) Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley
[kudu-CR] [doc] run doxygen with no warnings
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11784 ) Change subject: [doc] run doxygen with no warnings .. [doc] run doxygen with no warnings Updated doxygen configuration due to recent changes in Kudu C++ client sample application. Cleaned up other minor documentation issues in src/kudu/client/schema.h. With this patch, doxygen runs without any warnings while auto-generating documentation for Kudu C++ client API (verified with doxygen 1.8.14 built on OS X 10.11.6). This is a follow-up for a30c9211e8e73ead70acdb6108e5a0eac90cb5cb. Change-Id: Iedb31849288caa59456aff6eb075cf23c843d4e0 Reviewed-on: http://gerrit.cloudera.org:8080/11784 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M CMakeLists.txt M docs/support/doxygen/client_api.doxy.in M src/kudu/client/schema.h 3 files changed, 10 insertions(+), 1 deletion(-) Approvals: Kudu Jenkins: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11784 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iedb31849288caa59456aff6eb075cf23c843d4e0 Gerrit-Change-Number: 11784 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [client] expose location via internal API
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11679 ) Change subject: [client] expose location via internal API .. Patch Set 11: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11679 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib92cc6806073d32c859ae44ff803abb37cac99ac Gerrit-Change-Number: 11679 Gerrit-PatchSet: 11 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 25 Oct 2018 18:36:43 + Gerrit-HasComments: No
[kudu-CR](gh-pages) Updating web site for Kudu 1.8.0 release
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11772 ) Change subject: Updating web site for Kudu 1.8.0 release .. Patch Set 4: > > Patch Set 4: Code-Review+1 > > > > > > Patch Set 4: > > > > > > > > (1 comment) > > > > > > Go be honest, I'm not sure if it's a good idea to do that. I > mean > > > if someone builds the documentation on their own, it's going > to be > > > different from what's on the website and it's too late to cut > a new > > > RC as I archived 1.8.0 yesterday. > > > > > > On the other hand, I can see why it would be good as most devs > > > won't use their own doxygen build if it's publicly available. > > > > > > What does everyone think? I'm -0 on this. > > > > > > Anyway, Status ListPartitions() is not part of 1.8.0 and > backported > > > a30c921 on top of branch-1.8.x (https://gerrit.cloudera.org/c/11783) > > > so I can rebuild the docs if needed or at least it can be > fixed in > > > 1.8.1. > > > > Sorry for the trouble. I don't feel strong about the need to > rebuild the docs for 1.8.0: if fixing that in 1.8.1 is the easiest > way to go, that sounds good to me. > > I'm not really saying it's the easiest, as rebuilding the docs is > straight-forward enough, but I feel it's cleaner if the docs are > the same on the site and in the released sources. Right, I agree -- keeping things consistent is a noble goal. So, having those docs updated in 1.8.1 looks a better deal to me as well. Thanks! -- To view, visit http://gerrit.cloudera.org:8080/11772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: Iafad2ae1cc4ae5700025ea3167d51c6f886a2240 Gerrit-Change-Number: 11772 Gerrit-PatchSet: 4 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anonymous Coward (314) Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: helifu Gerrit-Comment-Date: Thu, 25 Oct 2018 18:37:16 + Gerrit-HasComments: No
[kudu-CR] [client] expose location via internal API
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11679 ) Change subject: [client] expose location via internal API .. Patch Set 11: > > Uploaded patch set 10. > > Could you re-base it on top of the main trunk's HEAD? > > Uploaded patch set 10. > > Could you re-base it on top of the main trunk's HEAD? Done. -- To view, visit http://gerrit.cloudera.org:8080/11679 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib92cc6806073d32c859ae44ff803abb37cac99ac Gerrit-Change-Number: 11679 Gerrit-PatchSet: 11 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 25 Oct 2018 18:35:45 + Gerrit-HasComments: No
[kudu-CR](gh-pages) Updating web site for Kudu 1.8.0 release
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11772 ) Change subject: Updating web site for Kudu 1.8.0 release .. Patch Set 4: > Patch Set 4: Code-Review+1 > > > > Patch Set 4: > > > > > > (1 comment) > > > > Go be honest, I'm not sure if it's a good idea to do that. I mean > > if someone builds the documentation on their own, it's going to be > > different from what's on the website and it's too late to cut a new > > RC as I archived 1.8.0 yesterday. > > > > On the other hand, I can see why it would be good as most devs > > won't use their own doxygen build if it's publicly available. > > > > What does everyone think? I'm -0 on this. > > > > Anyway, Status ListPartitions() is not part of 1.8.0 and backported > > a30c921 on top of branch-1.8.x (https://gerrit.cloudera.org/c/11783) > > so I can rebuild the docs if needed or at least it can be fixed in > > 1.8.1. > > Sorry for the trouble. I don't feel strong about the need to rebuild the > docs for 1.8.0: if fixing that in 1.8.1 is the easiest way to go, that sounds > good to me. I'm not really saying it's the easiest, as rebuilding the docs is straight-forward enough, but I feel it's cleaner if the docs are the same on the site and in the released sources. -- To view, visit http://gerrit.cloudera.org:8080/11772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: Iafad2ae1cc4ae5700025ea3167d51c6f886a2240 Gerrit-Change-Number: 11772 Gerrit-PatchSet: 4 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anonymous Coward (314) Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: helifu Gerrit-Comment-Date: Thu, 25 Oct 2018 18:35:26 + Gerrit-HasComments: No
[kudu-CR] [client] expose location via internal API
Alexey Serbin has uploaded a new patch set (#11) to the change originally created by Fengling Wang. ( http://gerrit.cloudera.org:8080/11679 ) Change subject: [client] expose location via internal API .. [client] expose location via internal API I added location info to client::KuduTabletServer and client::internal::RemoteTabletServer to enable the future work on location-aware C++ and Java clients. Change-Id: Ib92cc6806073d32c859ae44ff803abb37cac99ac --- M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/scan_token-internal.cc M src/kudu/client/tablet_server-internal.cc M src/kudu/client/tablet_server-internal.h 7 files changed, 34 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/11679/11 -- To view, visit http://gerrit.cloudera.org:8080/11679 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib92cc6806073d32c859ae44ff803abb37cac99ac Gerrit-Change-Number: 11679 Gerrit-PatchSet: 11 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Will Berkeley
[kudu-CR] [doc] run doxygen with no warnings
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11784 ) Change subject: [doc] run doxygen with no warnings .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11784 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iedb31849288caa59456aff6eb075cf23c843d4e0 Gerrit-Change-Number: 11784 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 25 Oct 2018 18:31:35 + Gerrit-HasComments: No
[kudu-CR](gh-pages) Updating web site for Kudu 1.8.0 release
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11772 ) Change subject: Updating web site for Kudu 1.8.0 release .. Patch Set 4: Code-Review+1 > > Patch Set 4: > > > > (1 comment) > > Go be honest, I'm not sure if it's a good idea to do that. I mean > if someone builds the documentation on their own, it's going to be > different from what's on the website and it's too late to cut a new > RC as I archived 1.8.0 yesterday. > > On the other hand, I can see why it would be good as most devs > won't use their own doxygen build if it's publicly available. > > What does everyone think? I'm -0 on this. > > Anyway, Status ListPartitions() is not part of 1.8.0 and backported > a30c921 on top of branch-1.8.x (https://gerrit.cloudera.org/c/11783) > so I can rebuild the docs if needed or at least it can be fixed in > 1.8.1. Sorry for the trouble. I don't feel strong about the need to rebuild the docs for 1.8.0: if fixing that in 1.8.1 is the easiest way to go, that sounds good to me. -- To view, visit http://gerrit.cloudera.org:8080/11772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: Iafad2ae1cc4ae5700025ea3167d51c6f886a2240 Gerrit-Change-Number: 11772 Gerrit-PatchSet: 4 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anonymous Coward (314) Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: helifu Gerrit-Comment-Date: Thu, 25 Oct 2018 18:25:25 + Gerrit-HasComments: No
[kudu-CR](branch-1.8.x) [c++ client] clean up inline doxygen documentation
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11783 ) Change subject: [c++ client] clean up inline doxygen documentation .. Patch Set 2: Code-Review+2 > Let me also cherry-pick 9db9021 in the 1.8.x branch as well -- that > precedes a30c921 and I don't see why we wouldn't want to have it in > 1.8.x as well. Woops, sorry -- a30c921 was about a method that didn't get into 1.8.x. Please ignore my prior comment. -- To view, visit http://gerrit.cloudera.org:8080/11783 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.8.x Gerrit-MessageType: comment Gerrit-Change-Id: Ib0038f45e746896b6abbcfaa5741760218d7a9ad Gerrit-Change-Number: 11783 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward (314) Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 25 Oct 2018 18:17:49 + Gerrit-HasComments: No
[kudu-CR](branch-1.8.x) [c++ client] clean up inline doxygen documentation
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11783 ) Change subject: [c++ client] clean up inline doxygen documentation .. Patch Set 2: Let me also cherry-pick 9db9021 in the 1.8.x branch as well -- that precedes a30c921 and I don't see why we wouldn't want to have it in 1.8.x as well. -- To view, visit http://gerrit.cloudera.org:8080/11783 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.8.x Gerrit-MessageType: comment Gerrit-Change-Id: Ib0038f45e746896b6abbcfaa5741760218d7a9ad Gerrit-Change-Number: 11783 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward (314) Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 25 Oct 2018 18:14:33 + Gerrit-HasComments: No
[kudu-CR] [examples] Add basic Spark example written in Scala
Mitch Barnett has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11788 Change subject: [examples] Add basic Spark example written in Scala .. [examples] Add basic Spark example written in Scala This patch adds a basic Kudu client that utilizes both Kudu Java APIs, as well as Spark SQL APIs. It will allow customers to pull down the pom.xml and scala source, then build and execute from their local machine. Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f --- A examples/scala/spark-example/README.adoc A examples/scala/spark-example/pom.xml A examples/scala/spark-example/src/main/scala/org/apache/kudu/examples/SparkExample.scala 3 files changed, 259 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/88/11788/1 -- To view, visit http://gerrit.cloudera.org:8080/11788 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I9ba09f0118c054a07b951e241c31d66245c57d3f Gerrit-Change-Number: 11788 Gerrit-PatchSet: 1 Gerrit-Owner: Mitch Barnett
[kudu-CR] [client] expose location via internal API
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11679 ) Change subject: [client] expose location via internal API .. Patch Set 10: > Uploaded patch set 10. Could you re-base it on top of the main trunk's HEAD? -- To view, visit http://gerrit.cloudera.org:8080/11679 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib92cc6806073d32c859ae44ff803abb37cac99ac Gerrit-Change-Number: 11679 Gerrit-PatchSet: 10 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 25 Oct 2018 14:21:20 + Gerrit-HasComments: No
[kudu-CR] [client] expose location via internal API
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11679 ) Change subject: [client] expose location via internal API .. Patch Set 10: Verified+1 Unrelated flake in MultiThreadedHybridClockTabletTest/0.UpdateNoMergeCompaction -- To view, visit http://gerrit.cloudera.org:8080/11679 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib92cc6806073d32c859ae44ff803abb37cac99ac Gerrit-Change-Number: 11679 Gerrit-PatchSet: 10 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 25 Oct 2018 14:18:35 + Gerrit-HasComments: No
[kudu-CR] [client] expose location via internal API
Alexey Serbin has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/11679 ) Change subject: [client] expose location via internal API .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/11679 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Ib92cc6806073d32c859ae44ff803abb37cac99ac Gerrit-Change-Number: 11679 Gerrit-PatchSet: 10 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Will Berkeley
[kudu-CR] [doc] run doxygen with no warnings
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11784 Change subject: [doc] run doxygen with no warnings .. [doc] run doxygen with no warnings Updated doxygen configuration due to recent changes in Kudu C++ client sample application. Cleaned up other minor documentation issues in src/kudu/client/schema.h. With this patch, doxygen runs without any warnings while auto-generating documentation for Kudu C++ client API (verified with doxygen 1.8.14 built on OS X 10.11.6). This is a follow-up for a30c9211e8e73ead70acdb6108e5a0eac90cb5cb. Change-Id: Iedb31849288caa59456aff6eb075cf23c843d4e0 --- M CMakeLists.txt M docs/support/doxygen/client_api.doxy.in M src/kudu/client/schema.h 3 files changed, 10 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/11784/1 -- To view, visit http://gerrit.cloudera.org:8080/11784 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iedb31849288caa59456aff6eb075cf23c843d4e0 Gerrit-Change-Number: 11784 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR](gh-pages) Updating web site for Kudu 1.8.0 release
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11772 ) Change subject: Updating web site for Kudu 1.8.0 release .. Patch Set 4: > Patch Set 4: > > (1 comment) Go be honest, I'm not sure if it's a good idea to do that. I mean if someone builds the documentation on their own, it's going to be different from what's on the website and it's too late to cut a new RC as I archived 1.8.0 yesterday. On the other hand, I can see why it would be good as most devs won't use their own doxygen build if it's publicly available. What does everyone think? I'm -0 on this. Anyway, Status ListPartitions() is not part of 1.8.0 and backported a30c921 on top of branch-1.8.x (https://gerrit.cloudera.org/c/11783) so I can rebuild the docs if needed or at least it can be fixed in 1.8.1. -- To view, visit http://gerrit.cloudera.org:8080/11772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: Iafad2ae1cc4ae5700025ea3167d51c6f886a2240 Gerrit-Change-Number: 11772 Gerrit-PatchSet: 4 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anonymous Coward (314) Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: helifu Gerrit-Comment-Date: Thu, 25 Oct 2018 07:02:58 + Gerrit-HasComments: No
[kudu-CR](gh-pages) Updating web site for Kudu 1.8.0 release
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/11772 ) Change subject: Updating web site for Kudu 1.8.0 release .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11772/4//COMMIT_MSG Commit Message: PS4: > Woops, I found some issues in the doxygen-generated docs for Kudu C++ clien replied on the main thread by mistake -- To view, visit http://gerrit.cloudera.org:8080/11772 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-MessageType: comment Gerrit-Change-Id: Iafad2ae1cc4ae5700025ea3167d51c6f886a2240 Gerrit-Change-Number: 11772 Gerrit-PatchSet: 4 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Anonymous Coward (314) Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: helifu Gerrit-Comment-Date: Thu, 25 Oct 2018 07:03:52 + Gerrit-HasComments: Yes
[kudu-CR](branch-1.8.x) [c++ client] clean up inline doxygen documentation
Hello Will Berkeley, Alexey Serbin, Kudu Jenkins, Adar Dembo, Anonymous Coward (314), I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11783 to look at the new patch set (#2). Change subject: [c++ client] clean up inline doxygen documentation .. [c++ client] clean up inline doxygen documentation This patch cleans up doxygen documentation in client.h. Most importantly, this patch removes the internal API methods (i.e. methods marked with KUDU_NO_EXPORT attribute) from the auto-generated documentation. Fixed other typos and mistakes in doxygen formatting, so now doxygen outputs less warnings. The rest of the doxygen warnings will be addressed in a follow-up patch. Change-Id: Ib0038f45e746896b6abbcfaa5741760218d7a9ad Reviewed-on: http://gerrit.cloudera.org:8080/11780 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo (cherry picked from commit a30c9211e8e73ead70acdb6108e5a0eac90cb5cb) --- M src/kudu/client/client.h 1 file changed, 53 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/83/11783/2 -- To view, visit http://gerrit.cloudera.org:8080/11783 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.8.x Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib0038f45e746896b6abbcfaa5741760218d7a9ad Gerrit-Change-Number: 11783 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward (314) Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley
[kudu-CR](branch-1.8.x) [c++ client] clean up inline doxygen documentation
Hello Will Berkeley, Alexey Serbin, Kudu Jenkins, Adar Dembo, Anonymous Coward (314), I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/11783 to review the following change. Change subject: [c++ client] clean up inline doxygen documentation .. [c++ client] clean up inline doxygen documentation This patch cleans up doxygen documentation in client.h. Most importantly, this patch removes the internal API methods (i.e. methods marked with KUDU_NO_EXPORT attribute) from the auto-generated documentation. Fixed other typos and mistakes in doxygen formatting, so now doxygen outputs less warnings. The rest of the doxygen warnings will be addressed in a follow-up patch. Change-Id: Ib0038f45e746896b6abbcfaa5741760218d7a9ad Reviewed-on: http://gerrit.cloudera.org:8080/11780 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo (cherry picked from commit a30c9211e8e73ead70acdb6108e5a0eac90cb5cb) --- M src/kudu/client/client.h 1 file changed, 68 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/83/11783/1 -- To view, visit http://gerrit.cloudera.org:8080/11783 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.8.x Gerrit-MessageType: newchange Gerrit-Change-Id: Ib0038f45e746896b6abbcfaa5741760218d7a9ad Gerrit-Change-Number: 11783 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Anonymous Coward (314) Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley
[kudu-CR] [client] expose location via internal API
Hello Will Berkeley, Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11679 to look at the new patch set (#10). Change subject: [client] expose location via internal API .. [client] expose location via internal API I added location info to client::KuduTabletServer and client::internal::RemoteTabletServer to enable the future work on location-aware C++ and Java clients. Change-Id: Ib92cc6806073d32c859ae44ff803abb37cac99ac --- M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/scan_token-internal.cc M src/kudu/client/tablet_server-internal.cc M src/kudu/client/tablet_server-internal.h 7 files changed, 30 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/11679/10 -- To view, visit http://gerrit.cloudera.org:8080/11679 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib92cc6806073d32c859ae44ff803abb37cac99ac Gerrit-Change-Number: 11679 Gerrit-PatchSet: 10 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley
[kudu-CR] [client] expose location via internal API
Hello Will Berkeley, Alexey Serbin, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11679 to look at the new patch set (#9). Change subject: [client] expose location via internal API .. [client] expose location via internal API I added location info to client::KuduTabletServer and client::internal::RemoteTabletServer to enable the future work on location-aware C++ and Java clients. Change-Id: Ib92cc6806073d32c859ae44ff803abb37cac99ac --- M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/meta_cache.cc M src/kudu/client/meta_cache.h M src/kudu/client/scan_token-internal.cc M src/kudu/client/tablet_server-internal.cc M src/kudu/client/tablet_server-internal.h 7 files changed, 29 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/11679/9 -- To view, visit http://gerrit.cloudera.org:8080/11679 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib92cc6806073d32c859ae44ff803abb37cac99ac Gerrit-Change-Number: 11679 Gerrit-PatchSet: 9 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2542: add initial authorization token impl
Andrew Wong has removed a vote on this change. Change subject: KUDU-2542: add initial authorization token impl .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/11750 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Id28747ec38675abdf50dce1e7c176d29213e370f Gerrit-Change-Number: 11750 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2542: add initial authorization token impl
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11750 ) Change subject: KUDU-2542: add initial authorization token impl .. Patch Set 3: Verified+1 Unrelated test failure: RebalanceParamTest.Rebalance/1 -- To view, visit http://gerrit.cloudera.org:8080/11750 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id28747ec38675abdf50dce1e7c176d29213e370f Gerrit-Change-Number: 11750 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 25 Oct 2018 06:26:14 + Gerrit-HasComments: No
[kudu-CR] [location awareness] Add ts location to TSInfoPB
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11727 ) Change subject: [location_awareness] Add ts location to TSInfoPB .. [location_awareness] Add ts location to TSInfoPB I added the 'location' field into TSInfoPB so that the tool client can get the ts location info. Change-Id: Ic5865cb47698817097a7f62c968c5cfd80e07259 Reviewed-on: http://gerrit.cloudera.org:8080/11727 Reviewed-by: Alexey Serbin Tested-by: Alexey Serbin --- M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/scripts/first_argument.sh M src/kudu/integration-tests/table_locations-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/master.proto 5 files changed, 118 insertions(+), 27 deletions(-) Approvals: Alexey Serbin: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11727 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ic5865cb47698817097a7f62c968c5cfd80e07259 Gerrit-Change-Number: 11727 Gerrit-PatchSet: 8 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley
[kudu-CR] [location awareness] Add ts location to TSInfoPB
Alexey Serbin has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/11727 ) Change subject: [location_awareness] Add ts location to TSInfoPB .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/11727 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Ic5865cb47698817097a7f62c968c5cfd80e07259 Gerrit-Change-Number: 11727 Gerrit-PatchSet: 7 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley
[kudu-CR] [location awareness] Add ts location to TSInfoPB
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11727 ) Change subject: [location_awareness] Add ts location to TSInfoPB .. Patch Set 7: Unrelated flake in org.apache.kudu.spark.kudu.DefaultSourceTest -- To view, visit http://gerrit.cloudera.org:8080/11727 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic5865cb47698817097a7f62c968c5cfd80e07259 Gerrit-Change-Number: 11727 Gerrit-PatchSet: 7 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 25 Oct 2018 06:19:01 + Gerrit-HasComments: No
[kudu-CR] [location awareness] Add ts location to TSInfoPB
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11727 ) Change subject: [location_awareness] Add ts location to TSInfoPB .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11727 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic5865cb47698817097a7f62c968c5cfd80e07259 Gerrit-Change-Number: 11727 Gerrit-PatchSet: 7 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 25 Oct 2018 06:18:53 + Gerrit-HasComments: No
[kudu-CR] [client] expose location via internal API
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11679 ) Change subject: [client] expose location via internal API .. Patch Set 8: > (1 comment) It seems IWYU is not happy: http://jenkins.kudu.apache.org/job/kudu-gerrit/15443/BUILD_TYPE=IWYU/artifact/build/latest/test-logs/iwyu.log Could you fix that, please? -- To view, visit http://gerrit.cloudera.org:8080/11679 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib92cc6806073d32c859ae44ff803abb37cac99ac Gerrit-Change-Number: 11679 Gerrit-PatchSet: 8 Gerrit-Owner: Fengling Wang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Fengling Wang Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Thu, 25 Oct 2018 06:03:20 + Gerrit-HasComments: No
