Skip to content

ModelProxy supports the "jobType" field of inference jobs - #118

Merged
zhogu merged 9 commits into
devfrom
zhogu/model-proxy-support-job-type
Nov 8, 2025
Merged

ModelProxy supports the "jobType" field of inference jobs#118
zhogu merged 9 commits into
devfrom
zhogu/model-proxy-support-job-type

Conversation

@zhogu

@zhogu zhogu commented Nov 5, 2025

Copy link
Copy Markdown
Contributor

This pull request refactors and improves the model server proxy logic in src/model-proxy/src/proxy/model_server.go, focusing on reliability, parallelism, and code simplification. Key changes include switching to a more robust job listing method, extracting forced job parameters, and parallelizing model mapping queries. Additionally, the handling of model keys in the deployment template has been updated.

Job listing and parameter extraction improvements:

  • Replaced the ListModelServingJobs function with ListInferenceJobs, which queries inference jobs specifically and uses a shared GETRequest helper for HTTP GETs. This also introduces a constant list of required job parameters and a dedicated GetJobParameters function to extract them. [1] [2] [3]
  • The job server URL and model listing logic now consistently use the new GETRequest helper, simplifying error handling and request construction.

Parallelization and concurrency:

  • The GetJobModelsMapping function now uses goroutines and a semaphore to parallelize requests for job server URLs and model lists, improving performance and reliability by avoiding bottlenecks and handling errors per job. Results are collected via a channel and aggregated after all jobs complete.

Deployment template update:

  • Removed the unused --modelkey argument from the model proxy deployment YAML template, reflecting the code changes and reducing configuration complexity.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the model proxy service to improve parallel processing, code reusability, and job filtering. The main changes include introducing concurrent job processing with goroutines, extracting common HTTP GET logic into a reusable function, and transitioning from tag-based job filtering to API-based job type filtering.

Key changes:

  • Introduced parallel processing of inference jobs using goroutines with semaphore-based concurrency control
  • Refactored HTTP GET logic into a reusable GETRequest function to eliminate code duplication
  • Changed from tag-based job filtering to API query parameter filtering (jobType=inference)

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/model-proxy/src/proxy/model_server.go Refactored HTTP requests, added concurrent job processing, introduced job parameter fetching, and replaced tag-based filtering with API-based filtering
src/model-proxy/deploy/model-proxy.yaml.template Removed deprecated --modelkey command-line argument
Comments suppressed due to low confidence (1)

src/model-proxy/src/proxy/model_server.go:37

  • The error message 'failed to GET jobs from %s' in the generic GETRequest function is too specific. This function is used for various endpoints (jobs, job config, job details, models), not just jobs. Consider a more generic message like 'failed to GET from %s' or 'HTTP GET request failed for %s'.
		return nil, fmt.Errorf("failed to GET jobs from %s: %w", url, err)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/model-proxy/src/proxy/model_server.go Outdated
Comment thread src/model-proxy/src/proxy/model_server.go Outdated
Comment thread src/model-proxy/src/proxy/model_server.go
Comment thread src/model-proxy/src/proxy/model_server.go
Comment thread src/model-proxy/src/proxy/model_server.go
Comment thread src/model-proxy/src/proxy/model_server.go Outdated
Comment thread src/model-proxy/src/proxy/model_server.go Outdated
Comment thread src/pylon/deploy/pylon-config/location.conf.template
@zhogu
zhogu merged commit bf4eef9 into dev Nov 8, 2025
4 of 5 checks passed
This was referenced Nov 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants