search

skyscrapr / terraform-provider-openai

Terraform Provider for OpenAI
MIT License
8 stars 3 forks source link

OpenAI Client Error Updating Assistant #89

Closed nathan-testeract closed 5 months ago

nathan-testeract commented 5 months ago

Hi,

Trying version 1.3.2 now and am getting this error when updating an assistant in place:

image

My tool resources look like this:

tool_resources = {
    file_search = {
      vector_store_ids = [
        openai_file.testeract-baseline-file-1.id,
        openai_file.testeract-baseline-file-2.id,
        openai_file.testeract-baseline-file-3.id,
        openai_file.testeract-baseline-file-4.id,
        openai_file.testeract-baseline-file-5.id,
        openai_file.testeract-baseline-file-6.id,
        openai_file.testeract-baseline-file-7.id,
        openai_file.testeract-baseline-file-8.id,
        openai_file.testeract-baseline-file-9.id,
        openai_file.testeract-baseline-file-10.id,
        openai_file.testeract-baseline-file-11.id,
        openai_file.testeract-baseline-file-12.id,
        openai_file.testeract-baseline-file-13.id,
        openai_file.testeract-baseline-file-14.id,
        openai_file.testeract-baseline-file-15.id,
        openai_file.testeract-baseline-file-16.id,
        openai_file.testeract-baseline-file-17.id,
        openai_file.testeract-baseline-file-18.id,
        openai_file.testeract-baseline-file-19.id,
        openai_file.testeract-baseline-file-20.id,
      ]
    }
  }

Using terraform 1.8.4

nathan-testeract commented 5 months ago

If I switch it up to this, which the docs somewhat suggest is possible:


tool_resources = {
    file_search = {
      vector_stores = {
        file_ids = [
          openai_file.testeract-baseline-file-1.id,
          openai_file.testeract-baseline-file-2.id,
          openai_file.testeract-baseline-file-3.id,
          openai_file.testeract-baseline-file-4.id,
          openai_file.testeract-baseline-file-5.id,
          openai_file.testeract-baseline-file-6.id,
          openai_file.testeract-baseline-file-7.id,
          openai_file.testeract-baseline-file-8.id,
          openai_file.testeract-baseline-file-9.id,
          openai_file.testeract-baseline-file-10.id,
          openai_file.testeract-baseline-file-11.id,
          openai_file.testeract-baseline-file-12.id,
          openai_file.testeract-baseline-file-13.id,
          openai_file.testeract-baseline-file-14.id,
          openai_file.testeract-baseline-file-15.id,
          openai_file.testeract-baseline-file-16.id,
          openai_file.testeract-baseline-file-17.id,
          openai_file.testeract-baseline-file-18.id,
          openai_file.testeract-baseline-file-19.id,
          openai_file.testeract-baseline-file-20.id,
        ]
      }
    }
  }
}

I get this:

image
skyscrapr commented 5 months ago

Ok, let me take a look. Probably just a restructure in the API call needed. Give me a few hours to have a look.

skyscrapr commented 5 months ago

I can see the issue. But it's a bigger change than expected. I'll need another day to resolve.

skyscrapr commented 5 months ago

Ok. Done the research. This change requires a new resource openai_vector_store.

Using the fileids option creates a vector store that then becomes unmanaged by terraform. So to handle this correctly requires adding a new resource and reference that using the vector_store_id

Pretty straight-forward. But needs a couple more days to implement and test.

skyscrapr commented 5 months ago

Ok, I've released v1.4.0 with support for a new resource openai_vector_store The docs are here

modifying your configuration to:

resource openai_vector_store testeract-baseline {
  file_ids = [
    openai_file.testeract-baseline-file-1.id,
    openai_file.testeract-baseline-file-2.id,
  ]
}

with a following 

tool_resources = {
    file_search = {
      vector_store_ids = [
        openai_vector_store.testeract-baseline.id
      ]
    }
  }
}

Let me know how it goes.

nathan-testeract commented 5 months ago

Hey, thank you for the fast response. I made the changes suggested but when apply them, run into this error:

openai_vector_store.testeract-baseline-vs: Creating...
╷
│ Error: Request cancelled
│
│ The plugin6.(*GRPCProvider).ApplyResourceChange request was cancelled.
╵

Stack trace from the terraform-provider-openai_v1.4.0 plugin:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1690cdb]

goroutine 21 [running]:
github.com/skyscrapr/terraform-provider-openai/openai.(*VectorStoreResource).Create.func1()
    github.com/skyscrapr/terraform-provider-openai/openai/vector_store_resource.go:164 +0x13b
github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry.RetryContext.func1()
    github.com/hashicorp/terraform-plugin-sdk/v2@v2.34.0/helper/retry/wait.go:30 +0x4e
github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry.(*StateChangeConf).WaitForStateContext.func1()
    github.com/hashicorp/terraform-plugin-sdk/v2@v2.34.0/helper/retry/state.go:113 +0x1b0
created by github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry.(*StateChangeConf).WaitForStateContext in goroutine 55
    github.com/hashicorp/terraform-plugin-sdk/v2@v2.34.0/helper/retry/state.go:86 +0x21a

Error: The terraform-provider-openai_v1.4.0 plugin crashed!

This is always indicative of a bug within the plugin. It would be immensely
helpful if you could report the crash with the plugin's maintainers so that it
can be fixed. The output above should help diagnose the issue.
skyscrapr commented 5 months ago

Can you paste your terraform here please?

nathan-testeract commented 5 months ago

Sure!

I have 20 of these file resources defined:


resource "openai_file" "testeract-baseline-file-1" {
  filepath = "../some-file.pdf"
}

Then these 2 other resources:


resource "openai_vector_store" "testeract-baseline-vs" {
  name = "testeract-baseline-vs"
  file_ids = [
    openai_file.testeract-baseline-file-1.id,
    openai_file.testeract-baseline-file-2.id,
    openai_file.testeract-baseline-file-3.id,
    openai_file.testeract-baseline-file-4.id,
    openai_file.testeract-baseline-file-5.id,
    openai_file.testeract-baseline-file-6.id,
    openai_file.testeract-baseline-file-7.id,
    openai_file.testeract-baseline-file-8.id,
    openai_file.testeract-baseline-file-9.id,
    openai_file.testeract-baseline-file-10.id,
    openai_file.testeract-baseline-file-11.id,
    openai_file.testeract-baseline-file-12.id,
    openai_file.testeract-baseline-file-13.id,
    openai_file.testeract-baseline-file-14.id,
    openai_file.testeract-baseline-file-15.id,
    openai_file.testeract-baseline-file-16.id,
    openai_file.testeract-baseline-file-17.id,
    openai_file.testeract-baseline-file-18.id,
    openai_file.testeract-baseline-file-19.id,
    openai_file.testeract-baseline-file-20.id,
  ]
}

resource "openai_assistant" "testeract-baseline" {
  name         = "testeract-baseline"
  description  = "description"
  model        = "gpt-4-turbo-preview"
  instructions = <<EOT
Blah blah blah
EOT
  temperature  = 1.0
  top_p        = 1.0

  tools = [
    {
      type = "code_interpreter"
    },
    {
      type = "file_search"
    }
  ]

  tool_resources = {
    file_search = {
      vector_store_ids = [
        openai_vector_store.testeract-baseline-vs.id
      ]
    }
  }
}
nathan-testeract commented 5 months ago

Trying to create an assistant from scratch fails with the same issue -- before I was trying to update an assistant in place.

skyscrapr commented 5 months ago

just released 1.4.1 with tests passing. Added a test for your scenario. Hopefully resolved now.

Run go test -v -cover ./openai/ === RUN TestAccAssistantResource_tool_simple --- PASS: TestAccAssistantResource_tool_simple (1.81s) === RUN TestAccAssistantResource_tool_code_interpreter --- PASS: TestAccAssistantResource_tool_code_interpreter (3.40s) === RUN TestAccAssistantResource_tool_function --- PASS: TestAccAssistantResource_tool_function (1.71s) === RUN TestAccAssistantResource_file_search --- PASS: TestAccAssistantResource_file_search (6.14s) === RUN TestAccAssistantResource_complex --- PASS: TestAccAssistantResource_complex (11.61s) === RUN TestAccFileDataSource --- PASS: TestAccFileDataSource (1.35s) === RUN TestAccFileResource --- PASS: TestAccFileResource (1.63s) === RUN TestAccFilesDataSource --- PASS: TestAccFilesDataSource (2.07s) === RUN TestAccFineTuneDataSource provider_test.go:28: env var TF_ACC_OPENAI not set. Skipping acceptance test due to cost or time --- SKIP: TestAccFineTuneDataSource (0.00s) === RUN TestAccFineTuningJobResource provider_test.go:28: env var TF_ACC_OPENAI not set. Skipping acceptance test due to cost or time --- SKIP: TestAccFineTuningJobResource (0.00s) === RUN TestAccFineTuningJobsDataSource --- PASS: TestAccFineTuningJobsDataSource (0.90s) === RUN TestAccModelDataSource --- PASS: TestAccModelDataSource (1.20s) === RUN TestAccModelsDataSource --- PASS: TestAccModelsDataSource (0.87s) === RUN TestAccVectorStoreResource --- PASS: TestAccVectorStoreResource (3.06s) PASS coverage: 58.9% of statements ok github.com/skyscrapr/terraform-provider-openai/openai 35.765s coverage: 58.9% of statements

please retest with latest version

nathan-testeract commented 5 months ago

Hi, thanks again, unfortunately, still getting an error when doing a fresh assistant creation + vector store + 20 files. 

╷
│ Error: Plugin did not respond
│
│ The plugin encountered an error, and failed to respond to the plugin6.(*GRPCProvider).ApplyResourceChange call. The plugin logs may contain more details.
╵

Stack trace from the terraform-provider-openai_v1.4.1.exe plugin:

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x0 pc=0xaaa380]

goroutine 374 [running]:
github.com/skyscrapr/terraform-provider-openai/openai.(*VectorStoreResource).Create.func1()
        github.com/skyscrapr/terraform-provider-openai/openai/vector_store_resource.go:164 +0x140
github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry.RetryContext.func1()
        github.com/hashicorp/terraform-plugin-sdk/v2@v2.34.0/helper/retry/wait.go:30 +0x4e
github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry.(*StateChangeConf).WaitForStateContext.func1()
        github.com/hashicorp/terraform-plugin-sdk/v2@v2.34.0/helper/retry/state.go:113 +0x1b0
created by github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry.(*StateChangeConf).WaitForStateContext in goroutine 356
        github.com/hashicorp/terraform-plugin-sdk/v2@v2.34.0/helper/retry/state.go:86 +0x21a

Error: The terraform-provider-openai_v1.4.1.exe plugin crashed!

This is always indicative of a bug within the plugin. It would be immensely
helpful if you could report the crash with the plugin's maintainers so that it
can be fixed. The output above should help diagnose the issue.

I made sure my OpenAI account had no vector stores, files or assistants already configured outside of TF. I'm running this on Windows if it matters, though can try a mac.

nathan-testeract commented 5 months ago

When applying, the provider does create the files and vector store, but does not associated the files with the store, nor create the assistant.

nathan-testeract commented 5 months ago

When destroying resources, I sometimes see:


╷
│ Error: OpenAI Client Error
│
│ Unable to delete File, got error: Delete "https://api.openai.com/v1/files/blahblahblah": context deadline exceeded (Client.Timeout exceeded while awaiting headers)```
nathan-testeract commented 5 months ago

Same result on a mac

nathan-testeract commented 5 months ago

@skyscrapr Ok -- I figured out some of the issue. It turns out vector stores can't handle .xlsx files and one of the files I was uploading was a spreadsheet. This caused an error that I guess the provider didn't handle. Once I converted the file to a pdf, the vector store gets create successfully.

But I did run into another error when creating the assistant. 

╷
│ Error: Provider produced inconsistent result after apply
│
│ When applying changes to openai_assistant.testeract-baseline, provider "provider[\"registry.terraform.io/skyscrapr/openai\"]" produced an unexpected new value: .temperature: was cty.NumberIntVal(1), but now null.
│
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.

In the assistant resource I had temperature = 1.0. Remove that line allows the assistant creation with a vector store to complete successfully.

nathan-testeract commented 5 months ago

It does appear like I can't change set/adjust the temperature or top_p values from the provider.

skyscrapr commented 5 months ago

Thanks for debugging this. I updated the test to use 20 files. But still cannot replicate your issue. However I added a bit more logic to make it a bit more robust. I don't think Windows will be making any difference in this case. I might need to look at how the error messages are being handled a bit more carefully.

The temperature thing looks like a bug and also affects top_p. Let me look into that one. It's a quick fix.

nathan-testeract commented 5 months ago

Yeah, pretty sure it was because one of my files wasn't a doc or pdf but an excel spreadsheet. After removing the spreadsheet from my file list, everything worked (minus the temperature setting.)

skyscrapr commented 5 months ago

Ok. I can see why the error isn't being returned correctly to let you know that's the issue. I've added a test to cover bad file types using xslx as a test.

I've also added support for temperature and top_p that I missed.

Tests are running now...

skyscrapr commented 5 months ago

ok 1.4.2 has been release that should return a good error message for unsupported files. And support for temperature and top_p should now be supported. Unsure about changes to temp or top_p, I need to dig into whether that's supported.

nathan-testeract commented 5 months ago

Thank you! Everything seems to be working as expected now.

skyscrapr commented 5 months ago

Good to hear. Thanks for the time spent troubleshooting.