Skip to content
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

Centralize server config handling #4154

Merged
merged 1 commit into from
May 6, 2024
Merged

Conversation

dhiltgen
Copy link
Collaborator

@dhiltgen dhiltgen commented May 4, 2024

This moves all the env var reading into one central module and logs the loaded config once at startup which should help in troubleshooting user server logs

Example server output

% ollama serve 2>&1 | head -1
2024/05/05 14:56:27 routes.go:989: INFO server config env="map[OLLAMA_DEBUG:false OLLAMA_LLM_LIBRARY: OLLAMA_MAX_LOADED_MODELS:1 OLLAMA_MAX_VRAM:0 OLLAMA_NOPRUNE:false OLLAMA_NUM_PARALLEL:1 OLLAMA_ORIGINS:[http://localhost https://localhost http://localhost:* https://localhost:* http://127.0.0.1 https://127.0.0.1 http://127.0.0.1:* https://127.0.0.1:* http://0.0.0.0 https://0.0.0.0 http://0.0.0.0:* https://0.0.0.0:*] OLLAMA_RUNNERS_DIR: OLLAMA_TMPDIR:]"

return strings.Trim(os.Getenv(key), "\"' ")
}

func initCfg() {
Copy link
Member

@jmorganca jmorganca May 5, 2024

Choose a reason for hiding this comment

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

This may be generally be an anti-pattern but could we use init() in this case? It might remove the need for the mutex since it the values will be initialized before the main function runs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wanted a bit more control on when the vars were loaded so we could have unit test cases that do os.Setenv(...) then grab init or call other routines that need variables init'd. If we use init() then it would read the var before we updated it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Refactored to do both - primarily init() driven with an exposed function to reload if we want to fiddle with env vars at runtime and have them respected.

@jmorganca
Copy link
Member

Thanks so much for doing this... some mostly cosmetic suggestions but this looks good

@dhiltgen dhiltgen force-pushed the central_config branch 2 times, most recently from dba60d5 to afae677 Compare May 5, 2024 22:08
Copy link
Member

@jmorganca jmorganca left a comment

Choose a reason for hiding this comment

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

A few more naming suggestions but otherwise looks good

server/ocfg/config.go Outdated Show resolved Hide resolved
server/ocfg/config.go Outdated Show resolved Hide resolved
server/ocfg/config.go Outdated Show resolved Hide resolved
server/ocfg/config.go Outdated Show resolved Hide resolved
server/ocfg/config.go Outdated Show resolved Hide resolved
This moves all the env var reading into one central module
and logs the loaded config once at startup which should
help in troubleshooting user server logs
@dhiltgen dhiltgen merged commit 840424a into ollama:main May 6, 2024
15 checks passed
@dhiltgen dhiltgen deleted the central_config branch May 6, 2024 00:08
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.

None yet

3 participants