imarkowitz commented on code in PR #52256: URL: https://github.com/apache/spark/pull/52256#discussion_r2356754559
########## python/pyspark/errors/exceptions/connect.py: ########## @@ -241,6 +260,15 @@ def getMessage(self) -> str: def getGrpcStatusCode(self) -> grpc.StatusCode: return self._grpc_status_code + def getBreakingChangeInfo(self) -> Optional[Dict[str, Any]]: Review Comment: We have a proto class defined in `sql/connect/common/src/main/protobuf/spark/connect/base.proto` but I didn't want to introduce that as a dependency here ########## sql/connect/common/src/main/protobuf/spark/connect/base.proto: ########## @@ -1095,6 +1095,32 @@ message FetchErrorDetailsResponse { // Portable error identifier across SQL engines // If null, error class or SQLSTATE is not set. optional string sql_state = 4; + + // Additional information if the error was caused by a breaking change. + optional BreakingChangeInfo breaking_change_info = 5; + } + + // BreakingChangeInfo defines the schema for breaking change information. + message BreakingChangeInfo { + // A message explaining how the user can migrate their job to work + // with the breaking change. + repeated string migration_message = 1; Review Comment: I think it's fine either way. The migration message is also returned as part of the error message, so I think that's the main way end users would interact with it. Instead of doing a conversion here I thought I would just return the value in the same format it's defined in. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org