Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14866 )
Change subject: [utility] auto-detection of cloud VM instance type ...................................................................... Patch Set 7: Code-Review+2 Having reviewed the code again, I noticed that none of the instance metadata subclasses actually need the expressivity that inheritance offers. All the overridden methods are just pass-through "return this flag", and the only one that deviates slightly (AWS's "get NTP server" method) can be modeled as a passthrough "return nullptr". What do you think of combining all of the classes into something more monolithic? I'm thinking: - A "registry" of sorts (vector of structs) that declaratively describes the properties of each cloud provider. I'm tempted to say it's static, but if that prevents gflag overrides from working properly, it can be constructed on the fly with each instantiation of the instance detector (since that's a rare operation). - That registry would be consumed at runtime by the instance detector, which would create a detection thread for each entry in the registry. - The detection thread would monolithically include the curl logic (because at this point there's no advantage to the separation). That should drastically reduce the amount of code/comments and make everything easier to follow, at the cost of lack of flexibility if down the line we do need the full expressivity of inheritance to control per-cloud behavior. Leaving you a +2 in case you'd rather keep the current approach. -- To view, visit http://gerrit.cloudera.org:8080/14866 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I083f4803decaef5d6e7d44184bbd98b074d2578b Gerrit-Change-Number: 14866 Gerrit-PatchSet: 7 Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Bankim Bhavsar <ban...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Greg Solovyev <gsolov...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Volodymyr Verovkin <verjov...@cloudera.com> Gerrit-Comment-Date: Wed, 22 Jan 2020 18:12:56 +0000 Gerrit-HasComments: No
