You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/11/03 14:01:25 UTC
[GitHub] [arrow] kylepbit opened a new pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client properties.
kylepbit opened a new pull request #8572:
URL: https://github.com/apache/arrow/pull/8572
PR for https://issues.apache.org/jira/browse/ARROW-10467
Add a CallOption for specifying arbitrary properties as headers for RPC calls.
Add a ServerMiddleware that will intercept and pass the properties to a handler
when registered via the FlightServer 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client properties.
Posted by GitBox <gi...@apache.org>.
lidavidm commented on pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#issuecomment-720715993
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kylepbit edited a comment on pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client properties.
Posted by GitBox <gi...@apache.org>.
kylepbit edited a comment on pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#issuecomment-721247330
Yes - do you have suggestions as to how you think that would be best done? Perhaps a CallOption to specify a header and give it a key and value where the value is serializable, and have a method that serializes the value that can be overridden if people wish to specialize? The server could register a set of headers that it would like to receive.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client properties.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#issuecomment-720634836
https://issues.apache.org/jira/browse/ARROW-10467
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kylepbit commented on pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client properties.
Posted by GitBox <gi...@apache.org>.
kylepbit commented on pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#issuecomment-721247330
Yes - do you have suggestions as to how you think that would be best done? Perhaps a CallOption to specify a header and give it a key and value where the value is serializable, and have a method that serializes the value that can be overridden if people wish to specialize?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm closed pull request #8572: ARROW-10467: [FlightRPC][Java] Add the ability to pass arbitrary client headers.
Posted by GitBox <gi...@apache.org>.
lidavidm closed pull request #8572:
URL: https://github.com/apache/arrow/pull/8572
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client properties.
Posted by GitBox <gi...@apache.org>.
lidavidm commented on pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#issuecomment-721207690
Ok. I think it'd make more sense to implement better support for headers in general, rather than have an interface to a single hardcoded header in Flight - anything higher-level (such as encoding an options map) can then be implemented in the application. Does that sound 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kylepbit commented on pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client headers.
Posted by GitBox <gi...@apache.org>.
kylepbit commented on pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#issuecomment-722171554
Regarding the comment about sessions and cookies - yes, there is definite interest in making sure that the work there is building on top of, instead of parallel to, one another.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kylepbit commented on a change in pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client headers.
Posted by GitBox <gi...@apache.org>.
kylepbit commented on a change in pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#discussion_r517544488
##########
File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/ServerHeaderHandler.java
##########
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.flight;
+
+import java.util.function.Consumer;
+
+/**
+ * Interface for server side property handling.
+ */
+public interface ServerHeaderHandler extends Consumer<CallHeaders> {
Review comment:
There already is: see https://github.com/apache/arrow/pull/8572/files#diff-b19522af7605b7cfe033ecc1300f2f6b0347d7eaba2969f0193086eb9751a546R102
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kylepbit commented on a change in pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client headers.
Posted by GitBox <gi...@apache.org>.
kylepbit commented on a change in pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#discussion_r517550295
##########
File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java
##########
@@ -351,6 +359,14 @@ public Builder headerAuthenticator(CallHeaderAuthenticator headerAuthenticator)
return this;
}
+ /**
+ * Set the header handler.
+ */
+ public Builder headerHandler(ServerHeaderHandler headerHandler) {
Review comment:
As mentioned in another comment, the intent is to handle all headers. You can interpret them as properties, but I think the name is reflective of intent.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kylepbit commented on a change in pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client headers.
Posted by GitBox <gi...@apache.org>.
kylepbit commented on a change in pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#discussion_r517620350
##########
File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/ServerHeaderHandler.java
##########
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.flight;
+
+import java.util.function.Consumer;
+
+/**
+ * Interface for server side property handling.
+ */
+public interface ServerHeaderHandler extends Consumer<CallHeaders> {
Review comment:
Not sure I understand the comment, are you simply saying you can't correlate the headers that are incoming with the RPC call that generated them?
##########
File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/ServerHeaderHandler.java
##########
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.flight;
+
+import java.util.function.Consumer;
+
+/**
+ * Interface for server side property handling.
+ */
+public interface ServerHeaderHandler extends Consumer<CallHeaders> {
Review comment:
Not sure I understand the comment, are you saying you can't correlate the headers that are incoming with the RPC call that generated 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kylepbit commented on a change in pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client headers.
Posted by GitBox <gi...@apache.org>.
kylepbit commented on a change in pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#discussion_r517544006
##########
File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/ServerHeaderMiddleware.java
##########
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.flight;
+
+/**
+ * Middleware that's used to extract and pass properties to the server during requests.
+ */
+public class ServerHeaderMiddleware implements FlightServerMiddleware {
Review comment:
I think the name is appropriate given that this is intended to handle all headers for calls, which could be considered properties depending on the FlightServer implementation, but not necessarily.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #8572: ARROW-10467: [FlightRPC][Java] Add the ability to pass arbitrary client headers.
Posted by GitBox <gi...@apache.org>.
lidavidm commented on pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#issuecomment-723090365
Thanks @kylepbit!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on a change in pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client headers.
Posted by GitBox <gi...@apache.org>.
lidavidm commented on a change in pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#discussion_r517054957
##########
File path: java/flight/flight-core/src/test/java/org/apache/arrow/flight/TestCallOptions.java
##########
@@ -64,6 +67,61 @@ public void underTimeout() {
});
}
+ @Test
+ public void singleProperty() {
+ final MetadataAdapter headers = new MetadataAdapter(new Metadata());
Review comment:
We probably need a CallHeaders implementation that doesn't come from the flight-grpc package, or there's no way to use HeaderCallOption.
##########
File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightConstants.java
##########
@@ -23,5 +23,6 @@
public interface FlightConstants {
String SERVICE = "arrow.flight.protocol.FlightService";
+ String PROPERTY_HEADER = "Arrow-Properties";
Review comment:
Let's rename this to something more specific (HEADERS_KEY?) and make it an instance of `FlightServerMiddleware.Key`. Let's also change the constant to something that's more clearly namespaced under arrow, e.g. "org.apache.arrow.flight.ServerHeaderHandler".
##########
File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/ServerHeaderHandler.java
##########
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.flight;
+
+import java.util.function.Consumer;
+
+/**
+ * Interface for server side property handling.
+ */
+public interface ServerHeaderHandler extends Consumer<CallHeaders> {
Review comment:
This interface confuses me, as it seems within an RPC handler, there's no way to get the headers associated with that particular call.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] keeratsingh commented on a change in pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client headers.
Posted by GitBox <gi...@apache.org>.
keeratsingh commented on a change in pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#discussion_r517052188
##########
File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightServer.java
##########
@@ -351,6 +359,14 @@ public Builder headerAuthenticator(CallHeaderAuthenticator headerAuthenticator)
return this;
}
+ /**
+ * Set the header handler.
+ */
+ public Builder headerHandler(ServerHeaderHandler headerHandler) {
Review comment:
Can we rename this to propertyHeaderHandler if we decide to rename `ServerHeaderHandler`
to `ServerPropertyHeaderHandler`?
##########
File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/ServerHeaderHandler.java
##########
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.flight;
+
+import java.util.function.Consumer;
+
+/**
+ * Interface for server side property handling.
+ */
+public interface ServerHeaderHandler extends Consumer<CallHeaders> {
Review comment:
I agree with @tifflhl's suggestion the name is too generic, would something like `ServerPropertyHeaderHandler` and `ClientPropertyHeaderHandler` be more appropriate similar to `ClientBearerHeaderHandler`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kylepbit commented on a change in pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client headers.
Posted by GitBox <gi...@apache.org>.
kylepbit commented on a change in pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#discussion_r517543577
##########
File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/ServerHeaderHandler.java
##########
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.flight;
+
+import java.util.function.Consumer;
+
+/**
+ * Interface for server side property handling.
+ */
+public interface ServerHeaderHandler extends Consumer<CallHeaders> {
Review comment:
This is not specific to properties - it's generally for handling all headers so I think the name fits.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client headers.
Posted by GitBox <gi...@apache.org>.
lidavidm commented on pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#issuecomment-722021593
Just a higher level comment - I see on ARROW-10428 cookie support is being worked on and ARROW-10427 is about session headers. Since cookies are a particular header and sessions are (usually) a particular cookie, is there any interest in making sure all these features work together and are built on top of each other?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kylepbit commented on pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client properties.
Posted by GitBox <gi...@apache.org>.
kylepbit commented on pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#issuecomment-720749732
The intent is to be able to provide preferences to requests that would not be part of the actual action. Ex context to run the action in, etc.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kylepbit commented on pull request #8572: ARROW-10467: [FlightRPC][Java] Add the ability to pass arbitrary client headers.
Posted by GitBox <gi...@apache.org>.
kylepbit commented on pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#issuecomment-722693533
@lidavidm any other comments on this?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] tifflhl commented on a change in pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client headers.
Posted by GitBox <gi...@apache.org>.
tifflhl commented on a change in pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#discussion_r517036715
##########
File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/ServerHeaderHandler.java
##########
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.flight;
+
+import java.util.function.Consumer;
+
+/**
+ * Interface for server side property handling.
+ */
+public interface ServerHeaderHandler extends Consumer<CallHeaders> {
Review comment:
Should we add a unit test that demonstrate a sample usage for this handler?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] tifflhl commented on a change in pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client headers.
Posted by GitBox <gi...@apache.org>.
tifflhl commented on a change in pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#discussion_r517037359
##########
File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/ServerHeaderMiddleware.java
##########
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.flight;
+
+/**
+ * Middleware that's used to extract and pass properties to the server during requests.
+ */
+public class ServerHeaderMiddleware implements FlightServerMiddleware {
Review comment:
Can we rename ServerHeaderMiddleware to ServerPropertyMiddleware? The current name is a bit too generic.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on a change in pull request #8572: ARROW-10467: [FlightRPC][Java] Add the ability to pass arbitrary client headers.
Posted by GitBox <gi...@apache.org>.
lidavidm commented on a change in pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#discussion_r518470154
##########
File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/HeaderCallOption.java
##########
@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.flight;
+
+import io.grpc.Metadata;
+import io.grpc.stub.AbstractStub;
+import io.grpc.stub.MetadataUtils;
+
+/**
+ * Method option for supplying headers to method calls.
+ */
+public class HeaderCallOption implements CallOptions.GrpcCallOption {
+ private final Metadata propertiesMetadata = new Metadata();
+
+ /**
+ * Header property constructor.
+ *
+ * @param headers the headers that should be sent across. If a header is a string, it should only be valid ASCII
+ * characters. Binary headers should end in "-bin".
+ */
+ public HeaderCallOption(CallHeaders headers) {
+ for (String key : headers.keys()) {
+ if (key.endsWith(Metadata.BINARY_HEADER_SUFFIX)) {
+ propertiesMetadata.put(
Review comment:
This discards secondary values of multi-valued headers.
##########
File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/FlightCallHeaders.java
##########
@@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.flight;
+
+import java.util.Collection;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Multimap;
+
+import io.grpc.Metadata;
+
+/**
+ * An implementation of the Flight headers interface for headers.
+ */
+public class FlightCallHeaders implements CallHeaders {
+ private final Multimap<String, Object> keysAndValues;
+
+ public FlightCallHeaders() {
+ this.keysAndValues = ArrayListMultimap.create();
+ }
+
+ @Override
+ public String get(String key) {
+ if (key.endsWith(Metadata.BINARY_HEADER_SUFFIX)) {
+ return new String((byte[]) Iterables.get(this.keysAndValues.get(key), 0));
+ }
+
+ return (String) Iterables.get(this.keysAndValues.get(key), 0);
Review comment:
Iterables.get appears to throw if there's no element; for consistency with MetadataAdapter this should return null when there's no element.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on a change in pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client headers.
Posted by GitBox <gi...@apache.org>.
lidavidm commented on a change in pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#discussion_r517682878
##########
File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/ServerHeaderHandler.java
##########
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.flight;
+
+import java.util.function.Consumer;
+
+/**
+ * Interface for server side property handling.
+ */
+public interface ServerHeaderHandler extends Consumer<CallHeaders> {
Review comment:
Yes - within the body of an RPC handler, how do I access the headers that the client sent for that call?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] tifflhl commented on a change in pull request #8572: ARROW-10467: [Java] Add the ability to pass arbitrary client headers.
Posted by GitBox <gi...@apache.org>.
tifflhl commented on a change in pull request #8572:
URL: https://github.com/apache/arrow/pull/8572#discussion_r517037479
##########
File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/ServerHeaderHandler.java
##########
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.arrow.flight;
+
+import java.util.function.Consumer;
+
+/**
+ * Interface for server side property handling.
+ */
+public interface ServerHeaderHandler extends Consumer<CallHeaders> {
Review comment:
Can we rename ServerHeaderHandler to ServerPropertyHandler? Like ServerHeaderMiddleware, the current name is a too generic.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org