Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/10291 )
Change subject: IMPALA-6948,IMPALA-6962: add end-to-end tests ...................................................................... Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/10291/3/be/src/catalog/catalog-server.cc File be/src/catalog/catalog-server.cc: http://gerrit.cloudera.org:8080/#/c/10291/3/be/src/catalog/catalog-server.cc@304 PS3, Line 304: void CatalogServer::CatalogUrlCallback(const Webserver::ArgumentMap& args, > did it this way to share the error handling and avoid multiple "error" fiel I just think it's weird that we don't show the dbs if there was an error with getting the catalog version. We return in L315 even though the dbs are just fine. Might make debugging slightly harder if something goes wrong. Don't think we need to do something too complicated, just want to be sure we can easily pinpoint the error if something does wrong. http://gerrit.cloudera.org:8080/#/c/10291/5/tests/custom_cluster/test_metadata_replicas.py File tests/custom_cluster/test_metadata_replicas.py: http://gerrit.cloudera.org:8080/#/c/10291/5/tests/custom_cluster/test_metadata_replicas.py@50 PS5, Line 50: self.client.execute("invalidate metadata functional.alltypes") check that the catalog version is at least 50 at this point to make debugging easier if something goes wrong (e.g. we purposely or accidentally change no-op ddls to not increment the version) -- To view, visit http://gerrit.cloudera.org:8080/10291 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6c5b39e29b2885cd30fede18833cbf23fb755f5 Gerrit-Change-Number: 10291 Gerrit-PatchSet: 5 Gerrit-Owner: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Wed, 09 May 2018 18:08:34 +0000 Gerrit-HasComments: Yes