-
Notifications
You must be signed in to change notification settings - Fork 23
Enable "Foundry Local" to run in local container environment #520
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
base: main
Are you sure you want to change the base?
Enable "Foundry Local" to run in local container environment #520
Conversation
| /// <summary> | ||
| /// Gets or sets the model ID of FoundryLocal. | ||
| /// </summary> | ||
| public string? ModelId { get; set; } |
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.
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.
Alias를 씁니다. 이건 이미 한참전부터 논의하고 결정한 거예요. 굳이 ModelId를 쓸 이유가 없습니다.
| { | ||
| private readonly AppSettings _appSettings = settings ?? throw new ArgumentNullException(nameof(settings)); | ||
|
|
||
| private const string ApiKey = "OPENAI_API_KEY"; |
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.
FoundryLocalManager에 있는 기본 값을 가져왔습니다.
public string ApiKey { get; internal set; } = "OPENAI_API_KEY";| } | ||
|
|
||
| if (string.IsNullOrWhiteSpace(settings.Alias!.Trim())) | ||
| if (settings.DisableFoundryLocalManager == true) |
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.
DisableFoundryLocalManager flag에 따라
분기를 가지도록 구조를 잡았습니다.
DisableFoundryLocalManager == true
=> Endpoint, ModelId 만 검증
DisableFoundryLocalManager == false
=> Alias 만 검증
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.
ModelId 쓰지 않습니다.
| string modelId; | ||
|
|
||
| if (settings!.DisableFoundryLocalManager == true) | ||
| { |
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.
DisableFoundryLocalManager flag에 따라
분기를 가지도록 구조를 잡았습니다.
DisableFoundryLocalManager == true
=> Endpoint, ModelId 만 사용
DisableFoundryLocalManager == false
=> Alias 만 사용
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.
ModelId 쓰지 않습니다
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.
ModelId 쓰지 않습니다
|
@name-of-okja 보통 새로운 속성을 도입할 때는 도입하는 이유에 대해 먼저 논의를 해야 합니다. 이미 히스토리가 있는 경우에는 더더욱 미리 찾아봐야 해요. 이미 appsettings.json 모델링 하면서 ModelId 를 사용하지 않는 이유에 대해 충분히 논의했더랬습니다. 그걸 먼저 찾아봤어야 했어요. |
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.
작업을 하기 전에 논의를 먼저 진행을 했었어야 했는데, 이 부분을 놓쳤습니다..!
다만,
이미 히스토리가 있는 경우에는 더더욱 미리 찾아봐야 해요. 이미 appsettings.json 모델링 하면서 ModelId 를 사용하지 않는 이유에 대해 충분히 논의했더랬습니다. 그걸 먼저 찾아봤어야 했어요.
히스토리를 찾아는 보았습니다.
샘플코드에서 Alias를 사용하는 이유는 GPU향 모델을 사용할지 CPU향 모델을 사용할지 Foundry Local이 알아서 결정하기 때문입니다.
당시 논의에서 Foundry Local이 GPU/CPU 모델 선택을 알아서 결정한다는 부분은 FoundryLocalManager를 사용하는 전제를 기반으로 한 것으로 이해했습니다.
현재는 Docker 환경이 추가되면서 FoundryLocalManager를 사용하지 않는 구조도 고려하고 있기 때문에, 당시 논의에서 다루지 않았던 상황이라고 판단하여 Model ID 속성을 추가해 보았습니다...
| ## Run in local container | ||
| 1. Set the Foundry Local service port. The default port OCP uses is `55438`. | ||
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.
Port가 랜덤으로 뽑히는 것 같습니다.
그래서 명시적으로 55438로 set 을 하는 가이드 문구를 추가 했습니다.
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.
- 명시적으로 55438 포트를 사용한다는 건 무엇을 근거로 하나요?
- 포트는
foundry service set --port같은 명령어를 이용하면 우리가 정할 수 있기는 합니다.
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.
명시적으로 55438 포트를 사용한다는 건 무엇을 근거로 하나요?
appsettings.josn의 기본 값인 55438 입니다.
"FoundryLocal": {
"Alias": "phi-4-mini",
"Endpoint": "http://127.0.0.1:55438/v1"
},foundry local startcommand 에서 나온 port를 입력해 주세요.- OCP의 Foundry Local Connector의 Port의 기본값은
55438로 할 테니 사용자 여러분께서 맞춰주세요.
이렇게 두 가지 방법으로 가이드를 줄 수 있을 것 같은데 후자가 조금 더 편할 것 같아서 foundry service set --port 로 했습니다.
그리고 다른 port를 사용하는 방법을 추가로 문서에 넣었습니다.
Alternatively, if you want to run with a different port, say `63997`, other than the default one, set it first by running the following command.
추가로 55438은 작업 당시에 foundry local start 에서 나온 랜덤으로 선택된 port 입니다.
| ``` | ||
| 1. Download the Foundry Local model. The default model OCP uses is `phi-4-mini`. | ||
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.
우선은 phi-4-mini로 모델을 받고, 가이드 문서 하단에서 foundry service list command를 통해서 Model ID를 가져오는 흐름으로 문서를 작성했습니다.
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.
우리는 ModelId를 사용하지 않습니다.
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.
Foundry Local SDK 예제 코드 에서도
var chatClient = client.GetChatClient(model?.ModelId); ModelId를 사용하고 있습니다.
만약 이와 같이 Alias를 사용하면, client.GetChatClient("phi-4-mini") 아래와 같은 에러가 발생합니다.
System.ClientModel.ClientResultException: Service request failed.
Status: 400 (Bad Request)
at OpenAI.ClientPipelineExtensions.ProcessMessageAsync(ClientPipeline pipeline, PipelineMessage message, RequestOptions options)
at OpenAI.Chat.ChatClient.CompleteChatAsync(BinaryContent content, RequestOptions options)
at OpenAI.Chat.ChatClient.<>c__DisplayClass19_0.<<CompleteChatStreamingAsync>b__0>d.MoveNext()
| ``` | ||
| 1. Run the app. The `{{Model ID}}` refers to the `Model ID` shown in the output of the `foundry service list` command. | ||
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.
Model ID를 가져다가, Alias 파라미터로 넘겨줘야 하는 느낌으로 작성했습니다.
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.
우리는 ModelId를 사용하지 않습니다.
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.
client.GetChatClient 메서드에는 파라미터로 model id를 받습니다만,
FoundryLocalManager을 사용하지 않고서는 Alais만으로 ModeId를 가져올 수가 없습니다.
| --alias {{Model ID}} ` | ||
| --endpoint http://host.docker.internal:55438/v1 ` | ||
| --disable-foundrylocal-manager | ||
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.
Alternatively, if you want to run with a different model, say
qwen2.5-7b, other than the default one, download it first by running the following command.
다른 모델 사용에 대한 예시에도 --alias {{Model ID}} 이거 말고, 생각이 안나서... 우선은 제외 했습니다..!
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.
- 우리는 ModelId를 사용하지 않습니다.
- 엔드포인트에 v1이 붙는 건 어떻게 확인하셨나요?
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.
우리는 ModelId를 사용하지 않습니다.
client.GetChatClient 메서드에는 파라미터로 model id를 받습니다만,
FoundryLocalManager을 사용하지 않고서는 Alais만으로 ModeId를 가져올 수가 없습니다.
엔드포인트에 v1이 붙는 건 어떻게 확인하셨나요?
FoundryLocalManager.cs에 이와 같이 있습니다.
public Uri Endpoint => new Uri(ServiceUri, "/v1");
| // Foundry Local | ||
| (ConnectorType.FoundryLocal, ArgumentOptionConstants.FoundryLocal.Alias, false), | ||
| (ConnectorType.FoundryLocal, ArgumentOptionConstants.FoundryLocal.Endpoint, false), | ||
| (ConnectorType.FoundryLocal, ArgumentOptionConstants.FoundryLocal.DisableFoundryLocalManager, false), |
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.
DisableFoundryLocalManager 값은 스위치입니다. 따라서 false가 아니라 true가 되어야 합니다.
|
|
||
| Console.WriteLine(" TBD"); | ||
| Console.WriteLine($" {ArgumentOptionConstants.FoundryLocal.Alias} The alias. Default to 'phi-4-mini'"); | ||
| Console.WriteLine($" {ArgumentOptionConstants.FoundryLocal.Endpoint} The endpoint URL. Default to 'http://127.0.0.1:55438/v1'"); |
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.
- 이 값은
DisableFoundryLocalManager값이 true 일 경우에만 필수입니다. 그 이외의 경우에는 필수가 아니예요. - BaseUrl 이 맞을까요, Endpoint가 맞을까요?
- 만약 Endpoint 라고 한다면 맨 뒤에
v1을 붙이는게 맞을까요, 아니면 커넥터를 생성하는 과정에서 동적으로 붙이는 게 맞을까요?
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.
BaseUrl 이 맞을까요, Endpoint가 맞을까요?
'http://127.0.0.1:55438/v1' 와 같이 v1 이 붙으면 Endpoint이고,
'http://127.0.0.1:55438' 와 같이 path가 없으면 BaseUrl이 맞을 것 같습니다.
만약 Endpoint 라고 한다면 맨 뒤에 v1을 붙이는게 맞을까요, 아니면 커넥터를 생성하는 과정에서 동적으로 붙이는 게 맞을까요?
Endpoint은 바뀔 수가 있으니
FoundryLocalManager.cs 와 같이 argument로 받을 때는 BaseUrl로 http://127.0.0.1:55438 이렇게만 받고, 커넥터를 생성하는 과정에서 동적으로 붙이는 게 맞는 것 같습니다!
BaseUrl로 수정 하겠습니다.
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.
이 값은 DisableFoundryLocalManager 값이 true 일 경우에만 필수입니다. 그 이외의 경우에는 필수가 아니예요.
** Foundry Local: **
--disable-foundrylocal-manager
Disable the built-in Foundry local manager.
When this flag is set, you must specify '--base-url'.
--base-url The base URL. Default to 'http://localhost:50594'
--alias The alias. Default to 'phi-4-mini'
| var chatClient = client.GetChatClient(modelId) | ||
| .AsIChatClient(); | ||
|
|
||
| Console.WriteLine($"The {this._appSettings.ConnectorType} connector created with model: {alias}"); | ||
| Console.WriteLine($"The {this._appSettings.ConnectorType} connector created with model: {modelId}"); |
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.
modelId 쓰지 않습니다.
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.
client.GetChatClient 에서 alias 값을 그대로 사용하면, 에러가 발생합니다.
System.ClientModel.ClientResultException: Service request failed.
Status: 400 (Bad Request)
at OpenAI.ClientPipelineExtensions.ProcessMessageAsync(ClientPipeline pipeline, PipelineMessage message, RequestOptions options)
at OpenAI.Chat.ChatClient.CompleteChatAsync(BinaryContent content, RequestOptions options)
at OpenAI.Chat.ChatClient.<>c__DisplayClass19_0.<<CompleteChatStreamingAsync>b__0>d.MoveNext()
| /// <summary> | ||
| /// Defines the constant for '--disable-foundrylocal-manager'. | ||
| /// </summary> | ||
| public const string DisableFoundryLocalManager = "--disable-foundrylocal-manager"; |
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.
이 값을 --disable-foundrylocal-manager로 할 것인지 --disable-foundry-local-manager로 할 것인지 --disable-flm`으로 할 것인지, 아니면 다 받아줄 것인지 등등에 대해 우리는 논의를 한 적이 한 번도 없습니다. 어떤 것이 가장 좋은 방법 같은가요?
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.
이러한 접근법으로 생각해도 되는지는 모르겠습니다만..
널널하게 여러 조건을 받아줄 경우에
만약 --disable-foundrylocal-manager, --disable-foundry-local-manager 이 두 개를 허용하면,
상수명을 생각하기 어려워서,
--disable-foundrylocal-manageror--disable-foundry-local-manager--disable-flm
이렇게 두 개만 받아주는 게 좋지 않을까 생각합니다.
허나 개인적으로는
--disable-flm flm을 보고 Foundry Local Manger를 추론하기도 어색하고, 추후에 Fast LLM Manager 같은 게 생긴다면, 오해의 소지도 있을 것 같습니다.
그리고
--disable-foundrylocal-manager,--disable-foundry-local-manager이 두 개를 허용하면,
상수명을 생각하기 어려워서,
그래서 개인적으로는 --disable-foundrylocal-manager, --disable-foundry-local-manager 둘 중 하나만 허용하는 게 좋을 것 같다고 생각합니다.
| case ArgumentOptionConstants.FoundryLocal.DisableFoundryLocalManager: | ||
| this.DisableFoundryLocalManager = true; |
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.
여기선 스위치로 처리했네요?
| "FoundryLocal": { | ||
| "Alias": "phi-4-mini" | ||
| "Alias": "phi-4-mini", | ||
| "Endpoint": "http://127.0.0.1:55438/v1" |
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.
127.0.0.1이 맞나요 localhost가 맞나요?
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.
localhost가 맞습니다!
| public class FoundryLocalConnectorTests | ||
| { | ||
| private const string Alias = "phi-4-mini"; | ||
| private const string Endpoint = "http://127.0.0.1:55438/v1"; |
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.
여기도 127.0.0.1이 맞나요, lcoalhost가 맞나요?
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.
이 부분도 localhost가 맞습니다!
|
|
||
| /// <summary> | ||
| /// Gets or sets the Endpoint of FoundryLocal. | ||
| /// </summary> | ||
| public string? Endpoint { get; set; } |
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.
다른 모델들을 보면 Endpoint/BaseUrl 이 Alias/Model 보다 먼저 나옵니다. 여기도 일관성을 맞춰야 하지 않을까요?
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.
반영하겠습니다!


Purpose
Does this introduce a breaking change?
Pull Request Type
What kind of change does this Pull Request introduce?
README updated?
The top-level readme for this repo contains a link to each sample in the repo. If you're adding a new sample did you update the readme?
How to Test