Skip to content
This repository was archived by the owner on Aug 1, 2025. It is now read-only.

OpenAI provider that sends requests though sourcegraph server#875

Merged
chwarwick merged 11 commits into
mainfrom
cw/azure-provider-update
Aug 31, 2023
Merged

OpenAI provider that sends requests though sourcegraph server#875
chwarwick merged 11 commits into
mainfrom
cw/azure-provider-update

Conversation

@chwarwick

@chwarwick chwarwick commented Aug 31, 2023

Copy link
Copy Markdown
Contributor

Adds a new openai provider that routes requests though the sourcegraph instance. Currently this reuses the prompt logic from the unstabl-azure-openai provider. This should enable working though the sg backend and then any prompt updates can be handled separately.

Test plan

ran locally with https://github.com/sourcegraph/sourcegraph/pull/56063

@chwarwick

Copy link
Copy Markdown
Contributor Author

@philipp-spiess do you have any suggestions here, from the testing I've done I have been getting reasonable single and multiline autocompletes most of the time.

I'm not sure if I want to replace the existing provider or add another one since there may be some trialing the current one.

@philipp-spiess philipp-spiess left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good overall - Pretty much like what we do for Fireworks now too. I don't really have any advice, if you think it would make sense to have a separate provider while we're transitioning, I don't see a problem with that either

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is now defined twice: once here and once in the createProvider as a default.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What's the model that is running? Do we know that on the client? This logging is mostly for Fireworks where we can run multiple models though so don't worry about it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The model name will be customer specific it's whatever they name it when they create it in azure. So I just threw a place holder in there. I think we will document that we've tested against gpt-35-turbo so maybe that is a better placeholder.

Comment on lines 73 to 74

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The values here have changed a lot, was this on purpose?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not intentional I copied a bunch over from the fireworks provider based on a comment you made on the original pr about making the prompt similar and didn't notice those values were different.

@chwarwick

Copy link
Copy Markdown
Contributor Author

Looks good overall - Pretty much like what we do for Fireworks now too. I don't really have any advice, if you think it would make sense to have a separate provider while we're transitioning, I don't see a problem with that either

Thank you. Any concerns if I call this the unstable-openai provider I don't see a reason it would be limited to just Azure OpenAI once I apply the backend changes to the non-azure version of the server side pieces.

@chwarwick chwarwick force-pushed the cw/azure-provider-update branch from 2ea2cef to ac8b8b2 Compare August 31, 2023 14:32
@chwarwick chwarwick marked this pull request as ready for review August 31, 2023 15:32
@chwarwick chwarwick changed the title update azure provider to send to run though sg server OpenAI provider that sends requests though sourcegraph server Aug 31, 2023
@chwarwick chwarwick linked an issue Aug 31, 2023 that may be closed by this pull request
@chwarwick chwarwick merged commit 22c8b0f into main Aug 31, 2023
@chwarwick chwarwick deleted the cw/azure-provider-update branch August 31, 2023 22:56
burmudar pushed a commit that referenced this pull request Nov 21, 2024
## Changes

Fixes sourcegraph/jetbrains#610
`ActionUpdateThread.OLD_EDT is deprecated and going to be removed soon`

Why we get that issue?

https://plugins.jetbrains.com/docs/intellij/basic-action-system.html#principal-implementation-overrides

https://github.com/JetBrains/intellij-community/blob/idea/233.14475.28/platform/editor-ui-api/src/com/intellij/openapi/actionSystem/ActionUpdateThread.java

New API was become required in 2024 EAP, and every IntelliJ `Action`
must define if `update` should be running on `EDT` or `BGT` thread.
Unfortunately that API was not present before `2022.2`, so we need to
upgrade a bit.

## Test plan

This change requires full manual QA
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Azure OpenAI on the back-end

2 participants