kou commented on code in PR #43537:
URL: https://github.com/apache/arrow/pull/43537#discussion_r1702355452
##########
python/pyarrow/_flight.pyx:
##########
@@ -855,10 +888,14 @@ cdef class FlightInfo(_Weakrefable):
the descriptor for this flight.
endpoints : list of FlightEndpoint
a list of endpoints where this flight is available.
- total_records : int
- the total records in this flight, or -1 if unknown
- total_bytes : int
- the total bytes in this flight, or -1 if unknown
+ total_records : int optional, default None
+ the total records in this flight, or -1 if unknown.
+ total_bytes : int optional, default None
+ the total bytes in this flight, or -1 if unknown.
Review Comment:
How about using `-1` not `None` as the default value because the document
says `-1 if unknown` not `None if unknown`?
##########
python/pyarrow/_flight.pyx:
##########
@@ -950,7 +1005,9 @@ cdef class FlightInfo(_Weakrefable):
f"descriptor={self.descriptor} "
f"endpoints={self.endpoints} "
f"total_records={self.total_records} "
- f"total_bytes={self.total_bytes}>")
+ f"total_bytes={self.total_bytes} "
+ f"ordered={'true' if self.ordered else 'false'} "
Review Comment:
I think that we can use `True`/`False` in Python.
##########
python/pyarrow/_flight.pyx:
##########
@@ -770,7 +801,9 @@ cdef class FlightEndpoint(_Weakrefable):
def __repr__(self):
return (f"<pyarrow.flight.FlightEndpoint ticket={self.ticket!r} "
- f"locations={self.locations!r}>")
+ f"locations={self.locations!r} "
+ f"expiration_time={self.expiration_time} "
+ f"app_metadata='{self.app_metadata.hex()}'>")
Review Comment:
I think that we don't need to use the same style as C++ in Python.
So we can remove `.hex()`.
@jorisvandenbossche What do you think about this?
##########
python/pyarrow/_flight.pyx:
##########
@@ -736,6 +743,12 @@ cdef class FlightEndpoint(_Weakrefable):
CLocation.Parse(tobytes(location)).Value(&c_location))
self.endpoint.locations.push_back(c_location)
+ if expiration_time is not None:
+ self.endpoint.expiration_time = time_point(
+ microseconds(expiration_time.cast(timestamp("us")).value))
Review Comment:
Is microseconds correct?
I think that we should use nanoseconds:
https://github.com/apache/arrow/blob/8068554a5d18129121f824978c7ee4a9c430640b/cpp/src/arrow/flight/types.h#L76-L88
--
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]
