-
Notifications
You must be signed in to change notification settings - Fork 8
Add support for running inference against the org endpoint #66
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
Conversation
3096d1c
to
68ce62d
Compare
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
This PR enables specifying an organization when running model inference by adding an --org
flag, refactors Azure client configuration to separate root and path, and updates tests to accommodate the new method signatures.
- Introduce
--org
flag in run and eval commands and propagate it to the client - Refactor
AzureClientConfig
to useInferenceRoot
+InferencePath
and updateGetChatCompletionStream
to build URLs with an optional org segment - Update mocks, interfaces, and tests for the new
org
parameter
Reviewed Changes
Copilot reviewed 36 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
internal/azuremodels/unauthenticated_client.go | Added org parameter to stubbed streaming method |
internal/azuremodels/types.go | Added Organization field to ChatCompletionOptions |
internal/azuremodels/mock_client.go | Updated mock signature and implementation to include org |
internal/azuremodels/client.go | Updated interface to accept org argument |
internal/azuremodels/azure_client_test.go | Switched tests to use InferenceRoot and updated calls |
internal/azuremodels/azure_client_config.go | Split default inference URL into root and path fields |
internal/azuremodels/azure_client.go | Implemented org-aware URL construction for inference calls |
cmd/run/run.go | Added --org flag, examples, and passed org to handler |
cmd/run/run_test.go | Updated run command tests to expect org parameter |
cmd/eval/eval.go | Added --org flag, examples, and passed org to handler |
cmd/eval/eval_test.go | Updated eval tests to expect org parameter |
Comments suppressed due to low confidence (4)
cmd/run/run.go:416
- The help text for the
--org
flag is missing a closing parenthesis. Consider updating it to:"Organization to attribute usage to (omitting will attribute usage to the current actor)"
.
cmd.Flags().String("org", "", "Organization to attribute usage to (omitting will attribute usage to the current actor")
cmd/eval/eval.go:134
- The help text for the
--org
flag is missing a closing parenthesis. Consider updating it to:"Organization to attribute usage to (omitting will attribute usage to the current actor)"
.
cmd.Flags().String("org", "", "Organization to attribute usage to (omitting will attribute usage to the current actor")
internal/azuremodels/azure_client_test.go:52
- In tests,
InferencePath
remains unset and defaults to empty, relying on the server handling/
. For clarity and future compatibility, explicitly setInferencePath: defaultInferencePath
in the test config.
cfg := &AzureClientConfig{InferenceRoot: testServer.URL}
internal/azuremodels/client.go:7
- The doc comment for
GetChatCompletionStream
should be updated to reflect the neworg
parameter in the method signature.
// GetChatCompletionStream returns a stream of chat completions using the given options.
3afdc58
to
a787251
Compare
This pull request introduces support for specifying an organization when running evaluations or inference against models, along with refactoring the Azure client configuration for improved flexibility. Key changes include adding the
--org
flag to relevant commands, updating the Azure client to dynamically construct inference URLs based on the organization, and modifying associated tests to accommodate the new functionality.