-
Notifications
You must be signed in to change notification settings - Fork 309
Codetrans: enable remote endpoints #2100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Codetrans: enable remote endpoints #2100
Conversation
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Add support for remote LLM endpoints in deployment configs and service orchestrator.
- New Docker Compose file (
compose_remote.yaml
) for routing requests to a remote LLM endpoint - Updated README with setup instructions for
REMOTE_ENDPOINT
andOPENAI_API_KEY
- Extended
code_translation.py
to align inputs/outputs and register a remote LLM service
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
CodeTrans/docker_compose/intel/cpu/xeon/compose_remote.yaml | New compose file enabling remote LLM endpoints via environment variables |
CodeTrans/docker_compose/intel/cpu/xeon/README.md | Added entry and instructions for compose_remote.yaml |
CodeTrans/code_translation.py | Added align_* methods and remote service registration in orchestrator |
Comments suppressed due to low confidence (6)
CodeTrans/docker_compose/intel/cpu/xeon/compose_remote.yaml:15
- Mapping a full URL via
${REMOTE_ENDPOINT}
intoLLM_SERVICE_HOST_IP
can cause the host field to include a scheme and path; consider introducing a dedicatedREMOTE_ENDPOINT_URL
variable and parsing its host, port, and scheme, or requireREMOTE_ENDPOINT
to supply only the hostname.
- LLM_SERVICE_HOST_IP=${REMOTE_ENDPOINT}
CodeTrans/docker_compose/intel/cpu/xeon/README.md:144
- The anchor link to the instructions uses
#running-llm-models-deployed-on-remote-servers-with-compose_remoteyaml
, but the header identifier should match the actual heading text (likely…-with-compose_remote.yaml
). Update the anchor or heading to ensure the link works.
| [compose_remote.yaml](./compose_remote.yaml) | The LLM used is hosted on a remote server and an endpoint is used to access this model. Additional environment variables need to be set before running. See [instructions](#running-llm-models-deployed-on-remote-servers-with-compose_remoteyaml) below. |
CodeTrans/code_translation.py:56
- [nitpick] This function lacks a docstring describing its purpose, parameters, and return value. Consider adding a docstring similar to
align_inputs
for consistency and clarity.
def align_outputs(self, data, cur_node, inputs, runtime_graph, llm_parameters_dict, **kwargs):
CodeTrans/code_translation.py:69
- [nitpick] Add a docstring to explain how this generator transforms streaming LLM responses into the expected
data:
chunk format.
def align_generator(self, gen, **kwargs):
CodeTrans/code_translation.py:21
- There are no tests covering the new alignment methods (
align_inputs
,align_outputs
,align_generator
) or the remote service registration; consider adding unit tests to validate that inputs/outputs are correctly formatted and the remote endpoint is configured properly.
LLM_MODEL_ID = os.getenv("LLM_MODEL_ID", "mistralai/Mistral-7B-Instruct-v0.3")
CodeTrans/code_translation.py:69
- [nitpick] The name
align_generator
is somewhat ambiguous; consider renaming it to something likeformat_streaming_response
to better convey its role in formatting streamed LLM responses.
def align_generator(self, gen, **kwargs):
f667608
to
fd7267a
Compare
tested and verified functionality of megaservice with remote endpoint using curl commands |
CI tests are still failing, but appears to not be related to this PR.
|
xeon test failed due to the env setup. Not related to the PR. failed to solve: process "/bin/sh -c uv pip install dist/*.whl" did not complete successfully: exit code: 2 I ran the test manually on a machine and it passes. |
@Spycsh @letonghan may I get your review for this PR before the code freeze this Friday? |
Hi @alexsin368 could you please fix the DCO issue? Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
ebad12d
to
aeb72ba
Compare
aeb72ba
to
2dabcb3
Compare
fixing the DCO resulted in lots of merge conflicts. will open a new PR: #2195 |
Description
Add support for OpenAI API remote endpoints.
Issues
#1973
Type of change
List the type of change like below. Please delete options that are not relevant.
Dependencies
None.
Tests
Tested megaservice functionality with remote endpoints via curl commands.
Plan to add/modify test script.