[PR #5473] [MERGED] fix: environ lookup #11794

Closed
opened 2026-04-12 23:39:05 -05:00 by GiteaMirror · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/ollama/ollama/pull/5473
Author: @mxyng
Created: 7/3/2024
Status: Merged
Merged: 7/31/2024
Merged by: @mxyng

Base: mainHead: mxyng/environ


📝 Commits (10+)

📊 Changes

27 files changed (+514 additions, -482 deletions)

View changed files

📝 api/client.go (+1 -7)
📝 api/client_test.go (+0 -3)
📝 app/lifecycle/logging.go (+1 -1)
📝 cmd/cmd.go (+1 -1)
📝 cmd/interactive.go (+1 -1)
📝 envconfig/config.go (+208 -284)
📝 envconfig/config_test.go (+211 -64)
📝 gpu/amd_linux.go (+4 -4)
📝 gpu/amd_windows.go (+1 -1)
📝 gpu/assets.go (+3 -3)
📝 gpu/gpu.go (+6 -6)
📝 integration/basic_test.go (+1 -8)
📝 integration/concurrency_test.go (+37 -33)
📝 integration/max_queue_test.go (+6 -8)
📝 llm/memory_test.go (+2 -2)
📝 llm/server.go (+4 -4)
📝 server/images.go (+2 -2)
📝 server/manifest_test.go (+0 -2)
📝 server/modelpath.go (+3 -9)
📝 server/modelpath_test.go (+0 -3)

...and 7 more files

📄 Description

current envconfig is configured on import by init(). this is problematic and error prone in tests which sometimes override configurations. which means the test must call envconfig.LoadConfig() otherwise it'll have side effects

e.g.

func TestRemoveModels(t *testing.T) {
    t.Setenv("OLLAMA_MODELS", t.TempDir())
    // envconfig.LoadConfig()

    RemoveModels()
}

this pattern is repeated in many tests but anyone writing a new test can easily miss it

this change proposes a new envconfig where configurations are functions rather than variables. this allows dynamic lookup of configurations while keeping the same-ish interface


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/ollama/ollama/pull/5473 **Author:** [@mxyng](https://github.com/mxyng) **Created:** 7/3/2024 **Status:** ✅ Merged **Merged:** 7/31/2024 **Merged by:** [@mxyng](https://github.com/mxyng) **Base:** `main` ← **Head:** `mxyng/environ` --- ### 📝 Commits (10+) - [`35b89b2`](https://github.com/ollama/ollama/commit/35b89b2eaba4ac6fc4ae1ba4bf2ec6c8bafe9529) rfc: dynamic environ lookup - [`4f1afd5`](https://github.com/ollama/ollama/commit/4f1afd575d1dfd803b0d9abb995862d61e8d0734) host - [`d1a5227`](https://github.com/ollama/ollama/commit/d1a5227cadf6ae736f8dd5cb9fb7452dd015f820) origins - [`66fe77f`](https://github.com/ollama/ollama/commit/66fe77f0841622054e29f5fd3d3643f514991004) models - [`55cd3dd`](https://github.com/ollama/ollama/commit/55cd3ddccac14d48f5f129ec35b3a109be215d01) bool - [`8570c1c`](https://github.com/ollama/ollama/commit/8570c1c0ef73e89448f6724645f56b9b10efef44) keepalive - [`e2c3f6b`](https://github.com/ollama/ollama/commit/e2c3f6b3e2de014656ab9ddffccf7b89d1bcc09e) string - [`0f19101`](https://github.com/ollama/ollama/commit/0f1910129f0a73c469ce2c012d39c8d98b79ef80) int - [`1954ec5`](https://github.com/ollama/ollama/commit/1954ec5917bf81ac743ba19bf0e7a6da47766778) uint64 - [`78140a7`](https://github.com/ollama/ollama/commit/78140a712ce8feac6fad2ae2c0043056f1a47fdc) cleanup tests ### 📊 Changes **27 files changed** (+514 additions, -482 deletions) <details> <summary>View changed files</summary> 📝 `api/client.go` (+1 -7) 📝 `api/client_test.go` (+0 -3) 📝 `app/lifecycle/logging.go` (+1 -1) 📝 `cmd/cmd.go` (+1 -1) 📝 `cmd/interactive.go` (+1 -1) 📝 `envconfig/config.go` (+208 -284) 📝 `envconfig/config_test.go` (+211 -64) 📝 `gpu/amd_linux.go` (+4 -4) 📝 `gpu/amd_windows.go` (+1 -1) 📝 `gpu/assets.go` (+3 -3) 📝 `gpu/gpu.go` (+6 -6) 📝 `integration/basic_test.go` (+1 -8) 📝 `integration/concurrency_test.go` (+37 -33) 📝 `integration/max_queue_test.go` (+6 -8) 📝 `llm/memory_test.go` (+2 -2) 📝 `llm/server.go` (+4 -4) 📝 `server/images.go` (+2 -2) 📝 `server/manifest_test.go` (+0 -2) 📝 `server/modelpath.go` (+3 -9) 📝 `server/modelpath_test.go` (+0 -3) _...and 7 more files_ </details> ### 📄 Description current envconfig is configured on import by `init()`. this is problematic and error prone in tests which sometimes override configurations. which means the test must call `envconfig.LoadConfig()` otherwise it'll have side effects e.g. ```go func TestRemoveModels(t *testing.T) { t.Setenv("OLLAMA_MODELS", t.TempDir()) // envconfig.LoadConfig() RemoveModels() } ``` this pattern is repeated in many tests but anyone writing a new test can easily miss it this change proposes a new envconfig where configurations are functions rather than variables. this allows dynamic lookup of configurations while keeping the same-ish interface --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-04-12 23:39:05 -05:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/ollama#11794