Fix bug around obtaining default values

This commit is contained in:
kardolus
2023-06-18 00:06:22 -04:00
parent 0c6e5928ac
commit 8d1eef1611
14 changed files with 62 additions and 92 deletions

View File

@@ -150,7 +150,7 @@ chatgpt --list-models
## Configuration
The chatGPT CLI uses a two-level configuration system. The default configuration is read from the
file `resources/config.yaml` located within the package. These default values are:
file `utils/constants.go` located within the package. These default values are:
```yaml
model: gpt-3.5-turbo

View File

@@ -30,19 +30,12 @@ type Client struct {
historyStore history.HistoryStore
}
func New(caller http.Caller, cs config.ConfigStore, hs history.HistoryStore) (*Client, error) {
cm, err := configmanager.New(cs)
if err != nil {
return nil, err
}
result := &Client{
Config: cm.Config,
func New(caller http.Caller, cs config.ConfigStore, hs history.HistoryStore) *Client {
return &Client{
Config: configmanager.New(cs).Config,
caller: caller,
historyStore: hs,
}
return result, nil
}
func (c *Client) WithCapacity(capacity int) *Client {

View File

@@ -466,7 +466,7 @@ func newClientFactory(mc *MockCaller, mcs *MockConfigStore, mhs *MockHistoryStor
URL: defaultURL,
CompletionsPath: defaultCompletionsPath,
ModelsPath: defaultModelsPath,
}, nil).Times(1)
}).Times(1)
return &clientFactory{
mockCaller: mc,
@@ -478,8 +478,7 @@ func newClientFactory(mc *MockCaller, mcs *MockConfigStore, mhs *MockHistoryStor
func (f *clientFactory) buildClientWithoutConfig() *client.Client {
f.mockConfigStore.EXPECT().Read().Return(types.Config{}, nil).Times(1)
c, err := client.New(f.mockCaller, f.mockConfigStore, f.mockHistoryStore)
Expect(err).NotTo(HaveOccurred())
c := client.New(f.mockCaller, f.mockConfigStore, f.mockHistoryStore)
return c.WithCapacity(50)
}
@@ -487,9 +486,8 @@ func (f *clientFactory) buildClientWithoutConfig() *client.Client {
func (f *clientFactory) buildClientWithConfig(config types.Config) *client.Client {
f.mockConfigStore.EXPECT().Read().Return(config, nil).Times(1)
c, err := client.New(f.mockCaller, f.mockConfigStore, f.mockHistoryStore)
Expect(err).NotTo(HaveOccurred())
c := client.New(f.mockCaller, f.mockConfigStore, f.mockHistoryStore)
return c.WithCapacity(50)
}

View File

@@ -50,12 +50,11 @@ func (mr *MockConfigStoreMockRecorder) Read() *gomock.Call {
}
// ReadDefaults mocks base method.
func (m *MockConfigStore) ReadDefaults() (types.Config, error) {
func (m *MockConfigStore) ReadDefaults() types.Config {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "ReadDefaults")
ret0, _ := ret[0].(types.Config)
ret1, _ := ret[1].(error)
return ret0, ret1
return ret0
}
// ReadDefaults indicates an expected call of ReadDefaults.

View File

@@ -63,10 +63,7 @@ func run(cmd *cobra.Command, args []string) error {
}
if cmd.Flag("set-model").Changed {
cm, err := configmanager.New(config.New())
if err != nil {
return err
}
cm := configmanager.New(config.New())
if err := cm.WriteModel(modelName); err != nil {
return err
@@ -85,15 +82,13 @@ func run(cmd *cobra.Command, args []string) error {
return nil
}
secret := viper.GetString(utils.OpenAPIKeyEnv)
secret := viper.GetString(utils.OpenAIKeyEnv)
if secret == "" {
return errors.New("missing environment variable: " + utils.OpenAPIKeyEnv)
return errors.New("missing environment variable: " + utils.OpenAIKeyEnv)
}
client, err := client.New(http.New().WithSecret(secret), config.New(), history.New())
if err != nil {
return err
}
client := client.New(http.New().WithSecret(secret), config.New(), history.New())
if modelName != "" {
client = client.WithModel(modelName)
}

View File

@@ -6,12 +6,11 @@ import (
"gopkg.in/yaml.v3"
"os"
"path/filepath"
"runtime"
)
type ConfigStore interface {
Read() (types.Config, error)
ReadDefaults() (types.Config, error)
ReadDefaults() types.Config
Write(types.Config) error
}
@@ -38,12 +37,14 @@ func (f *FileIO) Read() (types.Config, error) {
return parseFile(f.configFilePath)
}
func (f *FileIO) ReadDefaults() (types.Config, error) {
_, thisFile, _, _ := runtime.Caller(0)
configPath := filepath.Join(thisFile, "..", "..", "resources", "config.yaml")
return parseFile(configPath)
func (f *FileIO) ReadDefaults() types.Config {
return types.Config{
Model: utils.OpenAIModel,
MaxTokens: utils.OpenAIModelMaxTokens,
URL: utils.OpenAIURL,
CompletionsPath: utils.OpenAICompletionsPath,
ModelsPath: utils.OpenAIModelsPath,
}
}
func (f *FileIO) Write(config types.Config) error {

View File

@@ -1,7 +1,6 @@
package configmanager
import (
"fmt"
"github.com/kardolus/chatgpt-cli/config"
"github.com/kardolus/chatgpt-cli/types"
)
@@ -11,11 +10,8 @@ type ConfigManager struct {
Config types.Config
}
func New(cs config.ConfigStore) (*ConfigManager, error) {
c, err := cs.ReadDefaults()
if err != nil {
return nil, fmt.Errorf("error reading default values: %w", err)
}
func New(cs config.ConfigStore) *ConfigManager {
c := cs.ReadDefaults()
configured, err := cs.Read()
if err == nil {
@@ -36,7 +32,7 @@ func New(cs config.ConfigStore) (*ConfigManager, error) {
}
}
return &ConfigManager{configStore: cs, Config: c}, nil
return &ConfigManager{configStore: cs, Config: c}
}
func (c *ConfigManager) WriteModel(model string) error {

View File

@@ -52,11 +52,10 @@ func testConfig(t *testing.T, when spec.G, it spec.S) {
when("Constructing a new ConfigManager", func() {
it("applies the default configuration when user config is missing", func() {
mockConfigStore.EXPECT().ReadDefaults().Return(defaultConfig, nil).Times(1)
mockConfigStore.EXPECT().ReadDefaults().Return(defaultConfig).Times(1)
mockConfigStore.EXPECT().Read().Return(types.Config{}, errors.New("no such file")).Times(1)
subject, err := configmanager.New(mockConfigStore)
Expect(err).NotTo(HaveOccurred())
subject := configmanager.New(mockConfigStore)
Expect(subject.Config.Model).To(Equal(defaultModel))
Expect(subject.Config.MaxTokens).To(Equal(defaultMaxTokens))
@@ -67,11 +66,10 @@ func testConfig(t *testing.T, when spec.G, it spec.S) {
it("gives precedence to the user provided model", func() {
userModel := "the-model"
mockConfigStore.EXPECT().ReadDefaults().Return(defaultConfig, nil).Times(1)
mockConfigStore.EXPECT().ReadDefaults().Return(defaultConfig).Times(1)
mockConfigStore.EXPECT().Read().Return(types.Config{Model: userModel}, nil).Times(1)
subject, err := configmanager.New(mockConfigStore)
Expect(err).NotTo(HaveOccurred())
subject := configmanager.New(mockConfigStore)
Expect(subject.Config.Model).To(Equal(userModel))
Expect(subject.Config.MaxTokens).To(Equal(defaultMaxTokens))
@@ -82,11 +80,10 @@ func testConfig(t *testing.T, when spec.G, it spec.S) {
it("gives precedence to the user provided max-tokens", func() {
userMaxTokens := 42
mockConfigStore.EXPECT().ReadDefaults().Return(defaultConfig, nil).Times(1)
mockConfigStore.EXPECT().ReadDefaults().Return(defaultConfig).Times(1)
mockConfigStore.EXPECT().Read().Return(types.Config{MaxTokens: userMaxTokens}, nil).Times(1)
subject, err := configmanager.New(mockConfigStore)
Expect(err).NotTo(HaveOccurred())
subject := configmanager.New(mockConfigStore)
Expect(subject.Config.Model).To(Equal(defaultModel))
Expect(subject.Config.MaxTokens).To(Equal(userMaxTokens))
@@ -97,11 +94,10 @@ func testConfig(t *testing.T, when spec.G, it spec.S) {
it("gives precedence to the user provided URL", func() {
userURL := "the-user-url"
mockConfigStore.EXPECT().ReadDefaults().Return(defaultConfig, nil).Times(1)
mockConfigStore.EXPECT().ReadDefaults().Return(defaultConfig).Times(1)
mockConfigStore.EXPECT().Read().Return(types.Config{URL: userURL}, nil).Times(1)
subject, err := configmanager.New(mockConfigStore)
Expect(err).NotTo(HaveOccurred())
subject := configmanager.New(mockConfigStore)
Expect(subject.Config.Model).To(Equal(defaultModel))
Expect(subject.Config.MaxTokens).To(Equal(defaultMaxTokens))
@@ -112,11 +108,10 @@ func testConfig(t *testing.T, when spec.G, it spec.S) {
it("gives precedence to the user provided completions-path", func() {
completionsPath := "the-completions-path"
mockConfigStore.EXPECT().ReadDefaults().Return(defaultConfig, nil).Times(1)
mockConfigStore.EXPECT().ReadDefaults().Return(defaultConfig).Times(1)
mockConfigStore.EXPECT().Read().Return(types.Config{CompletionsPath: completionsPath}, nil).Times(1)
subject, err := configmanager.New(mockConfigStore)
Expect(err).NotTo(HaveOccurred())
subject := configmanager.New(mockConfigStore)
Expect(subject.Config.Model).To(Equal(defaultModel))
Expect(subject.Config.MaxTokens).To(Equal(defaultMaxTokens))
@@ -127,11 +122,10 @@ func testConfig(t *testing.T, when spec.G, it spec.S) {
it("gives precedence to the user provided models-path", func() {
modelsPath := "the-models-path"
mockConfigStore.EXPECT().ReadDefaults().Return(defaultConfig, nil).Times(1)
mockConfigStore.EXPECT().ReadDefaults().Return(defaultConfig).Times(1)
mockConfigStore.EXPECT().Read().Return(types.Config{ModelsPath: modelsPath}, nil).Times(1)
subject, err := configmanager.New(mockConfigStore)
Expect(err).NotTo(HaveOccurred())
subject := configmanager.New(mockConfigStore)
Expect(subject.Config.Model).To(Equal(defaultModel))
Expect(subject.Config.MaxTokens).To(Equal(defaultMaxTokens))
@@ -143,11 +137,10 @@ func testConfig(t *testing.T, when spec.G, it spec.S) {
when("WriteModel()", func() {
it("writes the expected config file", func() {
mockConfigStore.EXPECT().ReadDefaults().Return(defaultConfig, nil).Times(1)
mockConfigStore.EXPECT().ReadDefaults().Return(defaultConfig).Times(1)
mockConfigStore.EXPECT().Read().Return(defaultConfig, nil).Times(1)
subject, err := configmanager.New(mockConfigStore)
Expect(err).NotTo(HaveOccurred())
subject := configmanager.New(mockConfigStore)
modelName := "the-model"
subject.Config.Model = modelName

View File

@@ -50,12 +50,11 @@ func (mr *MockConfigStoreMockRecorder) Read() *gomock.Call {
}
// ReadDefaults mocks base method.
func (m *MockConfigStore) ReadDefaults() (types.Config, error) {
func (m *MockConfigStore) ReadDefaults() types.Config {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "ReadDefaults")
ret0, _ := ret[0].(types.Config)
ret1, _ := ret[1].(error)
return ret0, ret1
return ret0
}
// ReadDefaults indicates an expected call of ReadDefaults.

View File

@@ -27,14 +27,11 @@ func testContract(t *testing.T, when spec.G, it spec.S) {
it.Before(func() {
RegisterTestingT(t)
apiKey := os.Getenv(utils.OpenAPIKeyEnv)
apiKey := os.Getenv(utils.OpenAIKeyEnv)
Expect(apiKey).NotTo(BeEmpty())
restCaller = http.New().WithSecret(apiKey)
var err error
defaults, err = config.New().ReadDefaults()
Expect(err).NotTo(HaveOccurred())
defaults = config.New().ReadDefaults()
})
when("accessing the completion endpoint", func() {

View File

@@ -54,10 +54,7 @@ func runMockServer() error {
onceServe.Do(func() {
go func() {
defaults, err = config.New().ReadDefaults()
if err != nil {
return
}
defaults = config.New().ReadDefaults()
http.HandleFunc("/ping", getPing)
http.HandleFunc(defaults.CompletionsPath, postCompletions)

View File

@@ -177,7 +177,7 @@ func testIntegration(t *testing.T, when spec.G, it spec.S) {
Expect(err).NotTo(HaveOccurred())
Expect(os.Setenv("HOME", homeDir)).To(Succeed())
Expect(os.Setenv(utils.OpenAPIKeyEnv, "some-key")).To(Succeed())
Expect(os.Setenv(utils.OpenAIKeyEnv, "some-key")).To(Succeed())
})
it.After(func() {
@@ -186,7 +186,7 @@ func testIntegration(t *testing.T, when spec.G, it spec.S) {
})
it("throws an error when the API key is missing", func() {
Expect(os.Unsetenv(utils.OpenAPIKeyEnv)).To(Succeed())
Expect(os.Unsetenv(utils.OpenAIKeyEnv)).To(Succeed())
command := exec.Command(binaryPath, "some prompt")
session, err := gexec.Start(command, io.Discard, io.Discard)
@@ -195,11 +195,11 @@ func testIntegration(t *testing.T, when spec.G, it spec.S) {
Eventually(session).Should(gexec.Exit(exitFailure))
output := string(session.Out.Contents())
Expect(output).To(ContainSubstring(utils.OpenAPIKeyEnv))
Expect(output).To(ContainSubstring(utils.OpenAIKeyEnv))
})
it("should not require an API key for the --version flag", func() {
Expect(os.Unsetenv(utils.OpenAPIKeyEnv)).To(Succeed())
Expect(os.Unsetenv(utils.OpenAIKeyEnv)).To(Succeed())
command := exec.Command(binaryPath, "--version")
session, err := gexec.Start(command, io.Discard, io.Discard)
@@ -209,7 +209,7 @@ func testIntegration(t *testing.T, when spec.G, it spec.S) {
})
it("should not require an API key for the --clear-history flag", func() {
Expect(os.Unsetenv(utils.OpenAPIKeyEnv)).To(Succeed())
Expect(os.Unsetenv(utils.OpenAIKeyEnv)).To(Succeed())
command := exec.Command(binaryPath, "--clear-history")
session, err := gexec.Start(command, io.Discard, io.Discard)
@@ -230,7 +230,7 @@ func testIntegration(t *testing.T, when spec.G, it spec.S) {
})
it("should require the chatgpt-cli folder but not an API key for the --set-model flag", func() {
Expect(os.Unsetenv(utils.OpenAPIKeyEnv)).To(Succeed())
Expect(os.Unsetenv(utils.OpenAIKeyEnv)).To(Succeed())
command := exec.Command(binaryPath, "--set-model", "123")
session, err := gexec.Start(command, io.Discard, io.Discard)
@@ -240,7 +240,7 @@ func testIntegration(t *testing.T, when spec.G, it spec.S) {
output := string(session.Out.Contents())
Expect(output).To(ContainSubstring(".chatgpt-cli/config.yaml: no such file or directory"))
Expect(output).NotTo(ContainSubstring(utils.OpenAPIKeyEnv))
Expect(output).NotTo(ContainSubstring(utils.OpenAIKeyEnv))
})
it("should return the expected result for the --version flag", func() {

View File

@@ -1,5 +0,0 @@
model: gpt-3.5-turbo
max_tokens: 4096
url: https://api.openai.com
completions_path: /v1/chat/completions
models_path: /v1/models

View File

@@ -1,3 +1,10 @@
package utils
const OpenAPIKeyEnv = "OPENAI_API_KEY"
const (
OpenAIKeyEnv = "OPENAI_API_KEY"
OpenAIModel = "gpt-3.5-turbo"
OpenAIModelMaxTokens = 4096
OpenAIURL = "https://api.openai.com"
OpenAICompletionsPath = "/v1/chat/completions"
OpenAIModelsPath = "/v1/models"
)