lidavidm commented on a change in pull request #12669:
URL: https://github.com/apache/arrow/pull/12669#discussion_r830232938
##########
File path: cpp/src/arrow/flight/types.cc
##########
@@ -337,23 +366,40 @@ bool ActionType::Equals(const ActionType& other) const {
return type == other.type && description == other.description;
}
-Status MetadataRecordBatchReader::ReadAll(
- std::vector<std::shared_ptr<RecordBatch>>* batches) {
- FlightStreamChunk chunk;
+Status ResultStream::Next(std::unique_ptr<Result>* info) {
+ return Next().Value(info);
+}
+
+Status MetadataRecordBatchReader::Next(FlightStreamChunk* next) {
+ ARROW_ASSIGN_OR_RAISE(*next, Next());
+ return Status::OK();
Review comment:
nit: `return Next().Value(next);` should work too?
##########
File path: cpp/src/arrow/python/flight.cc
##########
@@ -206,11 +206,13 @@ PyFlightResultStream::PyFlightResultStream(PyObject*
generator,
generator_.reset(generator);
}
-Status PyFlightResultStream::Next(std::unique_ptr<arrow::flight::Result>*
result) {
- return SafeCallIntoPython([=] {
- const Status status = callback_(generator_.obj(), result);
+arrow::Result<std::unique_ptr<arrow::flight::Result>>
PyFlightResultStream::Next() {
+ return SafeCallIntoPython(
+ [=]() -> arrow::Result<std::unique_ptr<arrow::flight::Result>> {
+ std::unique_ptr<arrow::flight::Result> result;
+ const Status status = callback_(generator_.obj(), &result);
RETURN_NOT_OK(CheckPyError());
Review comment:
Needs RETURN_NOT_OK(status)
##########
File path: cpp/src/arrow/flight/types.h
##########
@@ -507,14 +533,26 @@ class ARROW_FLIGHT_EXPORT MetadataRecordBatchReader {
/// \brief Get the schema for this stream.
virtual arrow::Result<std::shared_ptr<Schema>> GetSchema() = 0;
+
/// \brief Get the next message from Flight. If the stream is
/// finished, then the members of \a FlightStreamChunk will be
/// nullptr.
- virtual Status Next(FlightStreamChunk* next) = 0;
+ virtual arrow::Result<FlightStreamChunk> Next() = 0;
+
+ ARROW_DEPRECATED("Deprecated in 8.0.0. Use Result-returning overload
instead.")
+ Status Next(FlightStreamChunk* next);
+
/// \brief Consume entire stream as a vector of record batches
- virtual Status ReadAll(std::vector<std::shared_ptr<RecordBatch>>* batches);
+ virtual arrow::Result<std::vector<std::shared_ptr<RecordBatch>>>
ToRecordBatches();
+
+ ARROW_DEPRECATED("Deprecated in 8.0.0. Use ToRecordBatches overload
instead.")
+ Status ReadAll(std::vector<std::shared_ptr<RecordBatch>>* batches);
+
/// \brief Consume entire stream as a Table
- virtual Status ReadAll(std::shared_ptr<Table>* table);
+ virtual arrow::Result<std::shared_ptr<Table>> ToTable();
+
+ ARROW_DEPRECATED("Deprecated in 8.0.0. Use ToTable overload instead.")
+ Status ReadAll(std::shared_ptr<Table>* table);
Review comment:
These are analogous to these APIs:
https://github.com/apache/arrow/blob/d45931160baa5618659edaedaa7b2ea5885ce678/cpp/src/arrow/record_batch.h#L299-L303
If we want to deprecate these, we should deprecate those as well I think
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
