Copilot commented on code in PR #465: URL: https://github.com/apache/opennlp-sandbox/pull/465#discussion_r3221274359
########## opennlp-grpc/examples/python-client/sentdetect_example.py: ########## @@ -0,0 +1,58 @@ +# 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. + +import grpc +import opennlp_pb2 +import opennlp_pb2_grpc + + +def run(): + # Create gRPC channel + channel = grpc.insecure_channel("localhost:7071") + stub = opennlp_pb2_grpc.SentenceDetectorServiceStub(channel) + + print("Connecting to OpenNLP gRPC server...") + + # Discover available models + response = stub.GetAvailableModels(opennlp_pb2.Empty()) + models = list(response.models) + + if not models: + print("No models available on server.") + return Review Comment: The example currently makes RPC calls without a deadline and without handling `grpc.RpcError`. If the server is unreachable or model discovery fails, this will raise a stack trace (and can also hang without a timeout), which conflicts with the PR goal of clear failure-state output. Add per-call timeouts (e.g., `timeout=...`) and wrap the `GetAvailableModels` / `sentDetect` calls in `try/except grpc.RpcError` to print actionable messages (status code/details) and exit cleanly. ########## opennlp-grpc/examples/python-client/sentdetect_example.py: ########## @@ -0,0 +1,58 @@ +# 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. + +import grpc +import opennlp_pb2 +import opennlp_pb2_grpc + + +def run(): + # Create gRPC channel + channel = grpc.insecure_channel("localhost:7071") + stub = opennlp_pb2_grpc.SentenceDetectorServiceStub(channel) + + print("Connecting to OpenNLP gRPC server...") + + # Discover available models + response = stub.GetAvailableModels(opennlp_pb2.Empty()) + models = list(response.models) + + if not models: + print("No models available on server.") + return + + model = models[0] + print(f"Using model: {model.name} ({model.hash})") + + # Input text + text = "The Apache OpenNLP project is great. It is now connected to Python!" + + # Create request + request = opennlp_pb2.SentDetectRequest( + sentence=text, + model_hash=model.hash + ) + + # Call service + result = stub.sentDetect(request) + + print("\nSentence Detection Result:") + for i, sentence in enumerate(result.values, 1): + print(f"{i}. {sentence}") Review Comment: The channel is never explicitly closed and the address is hard-coded. For example code that’s meant to be run repeatedly, prefer using a context manager (`with grpc.insecure_channel(...) as channel:`) so resources are released promptly, and consider allowing the target (host/port) to be overridden via an env var or CLI arg to avoid editing source when testing non-default server configs. ########## opennlp-grpc/examples/python-client/sentdetect_example.py: ########## @@ -0,0 +1,58 @@ +# 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. + +import grpc +import opennlp_pb2 +import opennlp_pb2_grpc + + +def run(): + # Create gRPC channel + channel = grpc.insecure_channel("localhost:7071") + stub = opennlp_pb2_grpc.SentenceDetectorServiceStub(channel) + + print("Connecting to OpenNLP gRPC server...") + + # Discover available models + response = stub.GetAvailableModels(opennlp_pb2.Empty()) + models = list(response.models) + + if not models: + print("No models available on server.") + return + + model = models[0] + print(f"Using model: {model.name} ({model.hash})") + + # Input text + text = "The Apache OpenNLP project is great. It is now connected to Python!" + + # Create request + request = opennlp_pb2.SentDetectRequest( + sentence=text, + model_hash=model.hash + ) + + # Call service + result = stub.sentDetect(request) Review Comment: The example currently makes RPC calls without a deadline and without handling `grpc.RpcError`. If the server is unreachable or model discovery fails, this will raise a stack trace (and can also hang without a timeout), which conflicts with the PR goal of clear failure-state output. Add per-call timeouts (e.g., `timeout=...`) and wrap the `GetAvailableModels` / `sentDetect` calls in `try/except grpc.RpcError` to print actionable messages (status code/details) and exit cleanly. ########## opennlp-grpc/examples/python-client/README.md: ########## @@ -1,3 +1,21 @@ +<!-- +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. +--> + + Review Comment: This line contains trailing whitespace. Please remove trailing spaces in Markdown files to avoid noisy diffs and formatting/tooling issues. -- 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]
