You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "aiguofer (via GitHub)" <gi...@apache.org> on 2023/10/18 19:27:28 UTC
[PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
aiguofer opened a new pull request, #38331:
URL: https://github.com/apache/arrow/pull/38331
Making necessary changes in Java to expose the newly added app_metadata.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #38331:
URL: https://github.com/apache/arrow/pull/38331#discussion_r1364553859
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -94,6 +95,23 @@ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoin
*/
public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoint> endpoints, long bytes,
long records, boolean ordered, IpcOption option) {
+ this(schema, descriptor, endpoints, bytes, records, ordered, option, "");
+ }
+
+ /**
+ * Constructs a new instance.
+ *
+ * @param schema The schema of the Flight
+ * @param descriptor An identifier for the Flight.
+ * @param endpoints A list of endpoints that have the flight available.
+ * @param bytes The number of bytes in the flight
+ * @param records The number of records in the flight.
+ * @param ordered Whether the endpoints in this flight are ordered.
+ * @param option IPC write options.
+ * @param appMetadata App metadata for this flight.
+ */
+ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoint> endpoints, long bytes,
Review Comment:
Let's keep things immutable, but a builder class is perhaps reasonable
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on code in PR #38331:
URL: https://github.com/apache/arrow/pull/38331#discussion_r1367516482
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightEndpoint.java:
##########
@@ -54,13 +56,27 @@ public FlightEndpoint(Ticket ticket, Location... locations) {
* Constructs a new endpoint with an expiration time.
*
* @param ticket A ticket that describe the key of a data stream.
+ * @param expirationTime (optional) When this endpoint expires.
* @param locations The possible locations the stream can be retrieved from.
*/
public FlightEndpoint(Ticket ticket, Instant expirationTime, Location... locations) {
+ this(ticket, expirationTime, "", locations);
+ }
+
+ /**
+ * Constructs a new endpoint with an expiration time.
+ *
+ * @param ticket A ticket that describe the key of a data stream.
+ * @param expirationTime (optional) When this endpoint expires.
+ * @param appMetadata (optional) Application metadata associated with this endpoint.
+ * @param locations The possible locations the stream can be retrieved from.
+ */
+ public FlightEndpoint(Ticket ticket, Instant expirationTime, String appMetadata, Location... locations) {
Review Comment:
Changed to private and added builder.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #38331:
URL: https://github.com/apache/arrow/pull/38331#discussion_r1367070509
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightEndpoint.java:
##########
@@ -54,13 +56,27 @@ public FlightEndpoint(Ticket ticket, Location... locations) {
* Constructs a new endpoint with an expiration time.
*
* @param ticket A ticket that describe the key of a data stream.
+ * @param expirationTime (optional) When this endpoint expires.
* @param locations The possible locations the stream can be retrieved from.
*/
public FlightEndpoint(Ticket ticket, Instant expirationTime, Location... locations) {
+ this(ticket, expirationTime, "", locations);
Review Comment:
Use `null`
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightEndpoint.java:
##########
@@ -39,6 +40,7 @@ public class FlightEndpoint {
private final List<Location> locations;
private final Ticket ticket;
private final Instant expirationTime;
+ private final String appMetadata;
Review Comment:
Let's use `byte[]` since it can be binary. (ArrowBuf would be better but it needs explicit closing.)
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -51,6 +51,7 @@ public class FlightInfo {
private final long records;
private final boolean ordered;
private final IpcOption option;
+ private final String appMetadata;
Review Comment:
Let's use `byte[]` here too
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightEndpoint.java:
##########
@@ -54,13 +56,27 @@ public FlightEndpoint(Ticket ticket, Location... locations) {
* Constructs a new endpoint with an expiration time.
*
* @param ticket A ticket that describe the key of a data stream.
+ * @param expirationTime (optional) When this endpoint expires.
* @param locations The possible locations the stream can be retrieved from.
*/
public FlightEndpoint(Ticket ticket, Instant expirationTime, Location... locations) {
+ this(ticket, expirationTime, "", locations);
+ }
+
+ /**
+ * Constructs a new endpoint with an expiration time.
+ *
+ * @param ticket A ticket that describe the key of a data stream.
+ * @param expirationTime (optional) When this endpoint expires.
+ * @param appMetadata (optional) Application metadata associated with this endpoint.
+ * @param locations The possible locations the stream can be retrieved from.
+ */
+ public FlightEndpoint(Ticket ticket, Instant expirationTime, String appMetadata, Location... locations) {
Review Comment:
We can omit that. A builder could be added.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #38331:
URL: https://github.com/apache/arrow/pull/38331#discussion_r1367316074
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightEndpoint.java:
##########
@@ -39,6 +40,7 @@ public class FlightEndpoint {
private final List<Location> locations;
private final Ticket ticket;
private final Instant expirationTime;
+ private final String appMetadata;
Review Comment:
base64, or just omit it (or you could try to decode as a string and fall back to base64)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on code in PR #38331:
URL: https://github.com/apache/arrow/pull/38331#discussion_r1364456410
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -94,6 +95,23 @@ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoin
*/
public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoint> endpoints, long bytes,
long records, boolean ordered, IpcOption option) {
+ this(schema, descriptor, endpoints, bytes, records, ordered, option, "");
+ }
+
+ /**
+ * Constructs a new instance.
+ *
+ * @param schema The schema of the Flight
+ * @param descriptor An identifier for the Flight.
+ * @param endpoints A list of endpoints that have the flight available.
+ * @param bytes The number of bytes in the flight
+ * @param records The number of records in the flight.
+ * @param ordered Whether the endpoints in this flight are ordered.
+ * @param option IPC write options.
+ * @param appMetadata App metadata for this flight.
+ */
+ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoint> endpoints, long bytes,
Review Comment:
Similar question... should I add some other constructors to set metadata or would it be better to add a `setMetadata` method? It seems a bit annoying to have to specify every single parameter to leverage this new field.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #38331:
URL: https://github.com/apache/arrow/pull/38331#issuecomment-1775609715
You'll want to rebase again, also unskip Java here:
https://github.com/apache/arrow/blob/6f8f34bd344c5cdf158aacb0215c3409d2996c5e/dev/archery/archery/integration/runner.py#L615-L619
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on code in PR #38331:
URL: https://github.com/apache/arrow/pull/38331#discussion_r1367516972
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightEndpoint.java:
##########
@@ -54,13 +57,22 @@ public FlightEndpoint(Ticket ticket, Location... locations) {
* Constructs a new endpoint with an expiration time.
*
* @param ticket A ticket that describe the key of a data stream.
+ * @param expirationTime (optional) When this endpoint expires.
* @param locations The possible locations the stream can be retrieved from.
*/
public FlightEndpoint(Ticket ticket, Instant expirationTime, Location... locations) {
Review Comment:
Should we deprecate this constructor in favor of the builder?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on code in PR #38331:
URL: https://github.com/apache/arrow/pull/38331#discussion_r1364733419
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -94,6 +95,23 @@ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoin
*/
public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoint> endpoints, long bytes,
long records, boolean ordered, IpcOption option) {
+ this(schema, descriptor, endpoints, bytes, records, ordered, option, "");
+ }
+
+ /**
+ * Constructs a new instance.
+ *
+ * @param schema The schema of the Flight
+ * @param descriptor An identifier for the Flight.
+ * @param endpoints A list of endpoints that have the flight available.
+ * @param bytes The number of bytes in the flight
+ * @param records The number of records in the flight.
+ * @param ordered Whether the endpoints in this flight are ordered.
+ * @param option IPC write options.
+ * @param appMetadata App metadata for this flight.
+ */
+ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoint> endpoints, long bytes,
Review Comment:
Great, added that.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #38331:
URL: https://github.com/apache/arrow/pull/38331#issuecomment-1783311137
After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 57f643c2cecca729109daae18c7a64f3a37e76e4.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/18131118255) has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38331:
URL: https://github.com/apache/arrow/pull/38331#issuecomment-1769185550
:warning: GitHub issue #38022 **has been automatically assigned in GitHub** to PR creator.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #38331:
URL: https://github.com/apache/arrow/pull/38331#discussion_r1368973651
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightEndpoint.java:
##########
@@ -54,13 +57,22 @@ public FlightEndpoint(Ticket ticket, Location... locations) {
* Constructs a new endpoint with an expiration time.
*
* @param ticket A ticket that describe the key of a data stream.
+ * @param expirationTime (optional) When this endpoint expires.
* @param locations The possible locations the stream can be retrieved from.
*/
public FlightEndpoint(Ticket ticket, Instant expirationTime, Location... locations) {
Review Comment:
Hmm, I think it's OK to keep it
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -94,6 +97,23 @@ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoin
*/
public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoint> endpoints, long bytes,
long records, boolean ordered, IpcOption option) {
+ this(schema, descriptor, endpoints, bytes, records, ordered, option, null);
+ }
+
+ /**
+ * Constructs a new instance.
+ *
+ * @param schema The schema of the Flight
+ * @param descriptor An identifier for the Flight.
+ * @param endpoints A list of endpoints that have the flight available.
+ * @param bytes The number of bytes in the flight
+ * @param records The number of records in the flight.
+ * @param ordered Whether the endpoints in this flight are ordered.
+ * @param option IPC write options.
+ * @param appMetadata Metadata to send along with the flight
+ */
+ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoint> endpoints, long bytes,
Review Comment:
I think it's OK to just keep it.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on code in PR #38331:
URL: https://github.com/apache/arrow/pull/38331#discussion_r1367317299
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightEndpoint.java:
##########
@@ -39,6 +40,7 @@ public class FlightEndpoint {
private final List<Location> locations;
private final Ticket ticket;
private final Instant expirationTime;
+ private final String appMetadata;
Review Comment:
`Arrays.toString(appMetadata)` seems like a good choice, but maybe a little odd. If the array is some sort of encoded object, this won't make any sense. If it's a UTF8 string object, each char will be separated by a comma which is strange.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on PR #38331:
URL: https://github.com/apache/arrow/pull/38331#issuecomment-1777687899
No worries! Started on this but the java setup is quite different.
For the Producer, should I extend `IntegrationProducer` or `NoOpFlightProducer`? I've seen instances of both (and then there's `FlightSqlScenarioProducer` which implements the whole thing).
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on code in PR #38331:
URL: https://github.com/apache/arrow/pull/38331#discussion_r1367517643
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightInfo.java:
##########
@@ -94,6 +97,23 @@ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoin
*/
public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoint> endpoints, long bytes,
long records, boolean ordered, IpcOption option) {
+ this(schema, descriptor, endpoints, bytes, records, ordered, option, null);
+ }
+
+ /**
+ * Constructs a new instance.
+ *
+ * @param schema The schema of the Flight
+ * @param descriptor An identifier for the Flight.
+ * @param endpoints A list of endpoints that have the flight available.
+ * @param bytes The number of bytes in the flight
+ * @param records The number of records in the flight.
+ * @param ordered Whether the endpoints in this flight are ordered.
+ * @param option IPC write options.
+ * @param appMetadata Metadata to send along with the flight
+ */
+ public FlightInfo(Schema schema, FlightDescriptor descriptor, List<FlightEndpoint> endpoints, long bytes,
Review Comment:
should I make this constructor private and deprecate other constructors in favor of the builder?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #38331:
URL: https://github.com/apache/arrow/pull/38331
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on PR #38331:
URL: https://github.com/apache/arrow/pull/38331#issuecomment-1778057388
Something interesting I noticed from the Go test... the `Ticket` on the FlightEndpoint can be null?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on code in PR #38331:
URL: https://github.com/apache/arrow/pull/38331#discussion_r1367317299
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightEndpoint.java:
##########
@@ -39,6 +40,7 @@ public class FlightEndpoint {
private final List<Location> locations;
private final Ticket ticket;
private final Instant expirationTime;
+ private final String appMetadata;
Review Comment:
`Arrays.toString(appMetadata)` seems like a good choice, but maybe a little odd if the metadata is an object converted to a bytearray.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on code in PR #38331:
URL: https://github.com/apache/arrow/pull/38331#discussion_r1364448959
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightEndpoint.java:
##########
@@ -54,13 +56,27 @@ public FlightEndpoint(Ticket ticket, Location... locations) {
* Constructs a new endpoint with an expiration time.
*
* @param ticket A ticket that describe the key of a data stream.
+ * @param expirationTime (optional) When this endpoint expires.
* @param locations The possible locations the stream can be retrieved from.
*/
public FlightEndpoint(Ticket ticket, Instant expirationTime, Location... locations) {
+ this(ticket, expirationTime, "", locations);
+ }
+
+ /**
+ * Constructs a new endpoint with an expiration time.
+ *
+ * @param ticket A ticket that describe the key of a data stream.
+ * @param expirationTime (optional) When this endpoint expires.
+ * @param appMetadata (optional) Application metadata associated with this endpoint.
+ * @param locations The possible locations the stream can be retrieved from.
+ */
+ public FlightEndpoint(Ticket ticket, Instant expirationTime, String appMetadata, Location... locations) {
Review Comment:
Should I add a constructor for `FlightEndpoint(Ticket ticket, String appMetadata, Location... locations)`?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on code in PR #38331:
URL: https://github.com/apache/arrow/pull/38331#discussion_r1364449979
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightEndpoint.java:
##########
@@ -54,13 +56,27 @@ public FlightEndpoint(Ticket ticket, Location... locations) {
* Constructs a new endpoint with an expiration time.
*
* @param ticket A ticket that describe the key of a data stream.
+ * @param expirationTime (optional) When this endpoint expires.
* @param locations The possible locations the stream can be retrieved from.
*/
public FlightEndpoint(Ticket ticket, Instant expirationTime, Location... locations) {
+ this(ticket, expirationTime, "", locations);
Review Comment:
It seems like the default value is `""` in the C implementation. Should I stick with that here?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #38331:
URL: https://github.com/apache/arrow/pull/38331#issuecomment-1777179038
Ah, sorry - then you need to implement the integration test scenario for Java to do the same thing as Go/C++: add an AppMetadataFlightInfoEndpointScenario.java at https://github.com/apache/arrow/tree/main/java/flight/flight-integration-tests/src/main/java/org/apache/arrow/flight/integration/tests following appMetadataFlightInfoEndpointScenarioTester in Go (https://github.com/apache/arrow/blob/5b9f4b976118052824c5aeaa2220886839b6f4be/go/arrow/internal/flight_integration/scenario.go#L1241)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on code in PR #38331:
URL: https://github.com/apache/arrow/pull/38331#discussion_r1367321711
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightEndpoint.java:
##########
@@ -39,6 +40,7 @@ public class FlightEndpoint {
private final List<Location> locations;
private final Ticket ticket;
private final Instant expirationTime;
+ private final String appMetadata;
Review Comment:
I'll go with base64 since that always works and it's easy enough to decode in case someone wants to see the stored value.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "aiguofer (via GitHub)" <gi...@apache.org>.
aiguofer commented on code in PR #38331:
URL: https://github.com/apache/arrow/pull/38331#discussion_r1367313814
##########
java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightEndpoint.java:
##########
@@ -39,6 +40,7 @@ public class FlightEndpoint {
private final List<Location> locations;
private final Ticket ticket;
private final Instant expirationTime;
+ private final String appMetadata;
Review Comment:
How should I handle a `byte[]` in the `toString` method?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #38331:
URL: https://github.com/apache/arrow/pull/38331#issuecomment-1777745932
Start with `NoOpFlightProducer` here. `IntegrationProducer` should probably be renamed since it's specific to the roundtrip integration tests
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-38022: [Java][FlightRPC] Expose app_metadata on FlightInfo and FlightEndpoint [arrow]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #38331:
URL: https://github.com/apache/arrow/pull/38331#issuecomment-1781043837
> the `Ticket` on the FlightEndpoint can be null
It's Protobuf, so any field can be omitted, but this is the same as having a ticket whose value is an empty string. I don't think we need to expose this separately in other languages.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org