Re: [PR] generic channel support for FlightClient [arrow-rs]
alamb merged PR #9933: URL: https://github.com/apache/arrow-rs/pull/9933 -- 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]
Re: [PR] generic channel support for FlightClient [arrow-rs]
alamb commented on PR #9933: URL: https://github.com/apache/arrow-rs/pull/9933#issuecomment-4399813192 The CI [failure](https://github.com/apache/arrow-rs/actions/runs/25507015410/job/74869860767?pr=9933) is not due to your changes (your changes just triggered the rebuild) so I'll just merge this PR in and we can sort out any additional failures as a follow on PR -- 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]
Re: [PR] generic channel support for FlightClient [arrow-rs]
rumenov commented on code in PR #9933: URL: https://github.com/apache/arrow-rs/pull/9933#discussion_r3202854176 ## arrow-flight/tests/client_interceptor.rs: ## @@ -0,0 +1,100 @@ +// 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. + +//! Integration test: a tonic [`Channel`] wrapped in an [`InterceptedService`] Review Comment: Done -- 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]
Re: [PR] generic channel support for FlightClient [arrow-rs]
alamb commented on PR #9933: URL: https://github.com/apache/arrow-rs/pull/9933#issuecomment-4397920471 The MSRV failure is unrelated to this PR. For more details: - https://github.com/apache/arrow-rs/issues/9938 -- 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]
Re: [PR] generic channel support for FlightClient [arrow-rs]
rumenov commented on PR #9933: URL: https://github.com/apache/arrow-rs/pull/9933#issuecomment-4397622243 > Thank you @rumenov -- I think this is technically an API change (as now FlightClient is templated) but a good one. > > I think we should move the test and ensure CI is passing, but then this looks good to me > > We are preparing a new major release with API changes soon. see here for more details > > * #9110 Thanks for the review! It is an API change but it is not a breaking API change. Everything should keep working as before -- 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]
Re: [PR] generic channel support for FlightClient [arrow-rs]
alamb commented on code in PR #9933: URL: https://github.com/apache/arrow-rs/pull/9933#discussion_r3201560441 ## arrow-flight/tests/client_interceptor.rs: ## @@ -0,0 +1,100 @@ +// 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. + +//! Integration test: a tonic [`Channel`] wrapped in an [`InterceptedService`] Review Comment: Can you please put this into the normal tests in `arrow-flight/src/client.rs` New test binaries slow down CI and consume more space (as they need to be linked and don't run in parallel with other 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
