Henry Robinson has posted comments on this change. Change subject: OPSAPS-32457: Fix CatalogService Thrift changes to be backwards compatible. ......................................................................
Patch Set 2: (5 comments) It looks as though this patch returns everything to use single objects in the response type, except for the function case where the caller uses the new 'api'. Is that necessary, or defensive against other issues of this type? http://gerrit.cloudera.org:8080/#/c/2455/2//COMMIT_MSG Commit Message: Line 10: servies services http://gerrit.cloudera.org:8080/#/c/2455/2/be/src/service/impala-server.cc File be/src/service/impala-server.cc: Line 1383: DCHECK I don't think this should be a DCHECK - do we want malformed RPCs to bring down debug clusters? Better to return a response with an explanation. http://gerrit.cloudera.org:8080/#/c/2455/2/common/thrift/CatalogService.thrift File common/thrift/CatalogService.thrift: Line 55: 4: optional CatalogObjects.TCatalogObject updated_catalog_object I suggest changing the name to updated_catalog_object_DEPRECATED. Won't affect BDR, but we will be able to immediately see any new code that relies on this. Line 60: removed_catalog_object ditto for naming http://gerrit.cloudera.org:8080/#/c/2455/2/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java: Line 1787: response.result.setUpdated_catalog_object(newTable); This response isn't related to function creation, right? Why do we need to go back to singletons (rather than lists) for this case? -- To view, visit http://gerrit.cloudera.org:8080/2455 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iec04d07c48d7159d2837667d7039046de126a3ad Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-2.5.0_5.7.0 Gerrit-Owner: Alex Behm <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Juan Yu <[email protected]> Gerrit-HasComments: Yes
