Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/20514 )
Change subject: KUDU-3507 Fix mini ranger port detection ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/20514/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20514/2//COMMIT_MSG@25 PS2, Line 25: If for some : reason an other port is opened > I just wanted to handle the case like the old code did. I don't know why it I think it makes sense to clarify what's the reason of that strange behavior: maybe, in reality it isn't what we think of it. At least, it'd try to do so if I had spent some time already on this. http://gerrit.cloudera.org:8080/#/c/20514/2/src/kudu/ranger-kms/mini_ranger_kms.cc File src/kudu/ranger-kms/mini_ranger_kms.cc: http://gerrit.cloudera.org:8080/#/c/20514/2/src/kudu/ranger-kms/mini_ranger_kms.cc@280 PS2, Line 280: RETURN_NOT_OK(WaitForTcpBind( : process_->pid(), : &port_, : adresses, : MonoDelta::FromSeconds(0))); > I just wanted to make it backward compatible. I'm not sure how that citation answers my question, at least I could not see the answer there. If the code looks bad, then it needs to be fixed. This code seems to be copy-pasted with some updates, so I'm not sure about the amount of truth in it :) http://gerrit.cloudera.org:8080/#/c/20514/2/src/kudu/util/test_util.h File src/kudu/util/test_util.h: http://gerrit.cloudera.org:8080/#/c/20514/2/src/kudu/util/test_util.h@194 PS2, Line 194: int pid = -1 > Client tests use this file too. Can we update client specific files to c++1 It's fine -- client tests use some C++17 features as well. We need to keep the exported API in C++98, but internally we can do whatever we want. And currently we use C++17 for Kudu code that's not exported. I don't think we need to update client-specific files to C++17, at least in this patch. Or what do you mean by updating '... the client-specific files to c++17'? -- To view, visit http://gerrit.cloudera.org:8080/20514 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89c3409e9efd53a821a6d4244a35564e134323bb Gerrit-Change-Number: 20514 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Martonka <zmarto...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Zoltan Martonka <zmarto...@cloudera.com> Gerrit-Comment-Date: Wed, 11 Oct 2023 16:50:08 +0000 Gerrit-HasComments: Yes
