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

remove vscode azure session #692

Merged
merged 2 commits into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 11 additions & 21 deletions docs/src/content/docs/getting-started/configuration.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,9 @@

## Azure OpenAI <a id="azure" href=""></a>

The [Azure OpenAI](https://learn.microsoft.com/en-us/azure/ai-services/openai/reference#chat-completions) provider, `azure` uses the `AZURE_OPENAI_...` environment variables.

Check failure on line 216 in docs/src/content/docs/getting-started/configuration.mdx

View workflow job for this annotation

GitHub Actions / build

Duplicate content found. The sentence "You can use a managed identity (recommended) or a API key to authenticate with the Azure OpenAI service." is repeated.
Copy link

Choose a reason for hiding this comment

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

Duplicate content found. The sentence "You can use a managed identity (recommended) or a API key to authenticate with the Azure OpenAI service." is repeated.

generated by pr-docs-review-commit duplicate_content

You can use a managed identity (recommended) or a API key to authenticate with the Azure OpenAI service.
You can use a managed identity (recommended) or a API key to authenticate with the Azure OpenAI service.
You can also use a service principal as documented in [automation](/genaiscript/getting-started/automating-scripts).

Check warning on line 218 in docs/src/content/docs/getting-started/configuration.mdx

View workflow job for this annotation

GitHub Actions / build

The link provided in "[automation](/genaiscript/getting-started/automating-scripts)" might be broken or incorrect. It should be verified if it correctly leads to the automation documentation.
Copy link

Choose a reason for hiding this comment

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

The link provided in "automation" might be broken or incorrect. It should be verified if it correctly leads to the automation documentation.

generated by pr-docs-review-commit broken_link


### Managed Identity (Entra ID)

Expand Down Expand Up @@ -262,7 +262,17 @@

<li>

Open a terminal and **login** with [Azure CLI](https://learn.microsoft.com/en-us/javascript/api/overview/azure/identity-readme?view=azure-node-latest#authenticate-via-the-azure-cli).

```sh
az login
```

Check notice on line 269 in docs/src/content/docs/getting-started/configuration.mdx

View workflow job for this annotation

GitHub Actions / build

Content has been relocated. Instructions for logging in with Azure CLI have been moved from a subsection into a list item.
Copy link

Choose a reason for hiding this comment

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

Content has been relocated. Instructions for logging in with Azure CLI have been moved from a subsection into a list item.

generated by pr-docs-review-commit content_relocation


</li>

<li>

Update the `model` field in the `script` function to match the model deployment name in your Azure resource.

Check failure on line 275 in docs/src/content/docs/getting-started/configuration.mdx

View workflow job for this annotation

GitHub Actions / build

Incorrect reference in the documentation. The `model` field should be updated to match the model deployment name, but the provided code snippet incorrectly references 'model: "***:deployment-id"' which is not a valid deployment name.
Copy link

Choose a reason for hiding this comment

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

Incorrect reference in the documentation. The model field should be updated to match the model deployment name, but the provided code snippet incorrectly references 'model: "azure:deployment-id"' which is not a valid deployment name.

generated by pr-docs-review-commit incorrect_reference


```js 'model: "azure:deployment-id"'
script({
Expand All @@ -273,30 +283,10 @@

</li>

<li>

Open a terminal and login to Azure. See Visual Studio or CLI instructions below.

</li>

</ol>

</Steps>

#### Visual Studio Code

Visual Studio Code will ask you to allow using the **Microsoft** account
and then will open a browser where you can choose the user or service principal.

#### CLI

Login with [Azure CLI](https://learn.microsoft.com/en-us/javascript/api/overview/azure/identity-readme?view=azure-node-latest#authenticate-via-the-azure-cli)
then use the [cli](/genaiscript/reference/cli) as usual.

```sh
az login
```

### API Key

<Steps>
Expand Down
5 changes: 5 additions & 0 deletions packages/cli/src/nodehost.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,11 @@
if (!this._azureToken) throw new Error("Azure token not available")
tok.token = "Bearer " + this._azureToken.token
}
if (!tok) {
const { provider } = parseModelIdentifier(modelId)
if (provider === MODEL_PROVIDER_AZURE)
throw new Error("Azure end point not configured")
}

Check failure on line 182 in packages/cli/src/nodehost.ts

View workflow job for this annotation

GitHub Actions / build

The `tok` object is not checked for null or undefined before accessing its `token` property. This could lead to a runtime error if `tok` is null or undefined. Consider adding a null check before accessing the `token` property. πŸ› οΈ
Copy link

Choose a reason for hiding this comment

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

The error "Azure end point not configured" is thrown but not caught. This could lead to unexpected application termination. Consider handling this error to improve the application's robustness. πŸ› οΈ

generated by pr-review-commit unhandled_error

Copy link

Choose a reason for hiding this comment

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

The tok object is not checked for null or undefined before accessing its token property. This could lead to a runtime error if tok is null or undefined. Consider adding a null check before accessing the token property. πŸ› οΈ

generated by pr-review-commit missing_check

if (!tok && this.clientLanguageModel) {
return <LanguageModelConfiguration>{
model: modelId,
Expand Down
18 changes: 0 additions & 18 deletions packages/core/src/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -361,22 +361,4 @@ export async function updateConnectionConfiguration(
if (!content.includes(DOT_ENV_FILENAME))
await writeText(".gitignore", content + `\n${DOT_ENV_FILENAME}\n`)
}

// update .env
const { config, model } = dotEnvTemplate(provider, apiType)
let src = config
const current = await tryReadText(DOT_ENV_FILENAME)
if (current) {
if (!current.includes("GENAISCRIPT_DEFAULT_MODEL"))
src =
dedent`

## GenAIScript defaults
GENAISCRIPT_DEFAULT_MODEL="${model}"
# GENAISCRIPT_DEFAULT_TEMPERATURE=${DEFAULT_TEMPERATURE}

` + src
src = current + "\n" + src
}
await writeText(DOT_ENV_FILENAME, src)
}
53 changes: 0 additions & 53 deletions packages/vscode/src/azuremanager.ts

This file was deleted.

12 changes: 8 additions & 4 deletions packages/vscode/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,17 @@ import { activateTestController } from "./testcontroller"
import { activateDocsNotebook } from "./docsnotebook"
import { activateTraceTreeDataProvider } from "./tracetree"
import { registerCommand } from "./commands"
import { EXTENSION_ID, TOOL_NAME } from "../../core/src/constants"
import {
DOCS_CONFIGURATION_URL,
EXTENSION_ID,
TOOL_NAME,
} from "../../core/src/constants"
import type MarkdownIt from "markdown-it"
import MarkdownItGitHubAlerts from "markdown-it-github-alerts"
import { activateConnectionInfoTree } from "./connectioninfotree"
import { updateConnectionConfiguration } from "../../core/src/connection"
import { APIType } from "../../core/src/host"
import { openUrlInTab } from "./browser"

export async function activate(context: ExtensionContext) {
const state = new ExtensionState(context)
Expand All @@ -37,10 +42,9 @@ export async function activate(context: ExtensionContext) {
"genaiscript.connection.configure",
async (provider?: string, apiType?: APIType) => {
await updateConnectionConfiguration(provider, apiType)
const doc = await vscode.workspace.openTextDocument(
state.host.toUri("./.env")
await vscode.env.openExternal(
vscode.Uri.parse(DOCS_CONFIGURATION_URL)
)
await vscode.window.showTextDocument(doc)
}
),
registerCommand("genaiscript.request.abort", async () => {
Expand Down
18 changes: 13 additions & 5 deletions packages/vscode/src/lmaccess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
MODEL_PROVIDER_OPENAI,
MODEL_PROVIDER_CLIENT,
MODEL_PROVIDER_GITHUB,
TOOL_NAME,
} from "../../core/src/constants"
import { APIType } from "../../core/src/host"
import { parseModelIdentifier } from "../../core/src/models"
Expand Down Expand Up @@ -143,11 +144,18 @@

if (res.model) return res.model
else {
await vscode.commands.executeCommand(
"genaiscript.connection.configure",
res.provider,
res.apiType
)
const configure = "Configure..."
vscode.window
.showWarningMessage(
`${TOOL_NAME} - model connection not configured.`,
configure
)
.then((res) => {
if (res === configure)
vscode.commands.executeCommand(
"genaiscript.connection.configure"
)
})

Check failure on line 158 in packages/vscode/src/lmaccess.ts

View workflow job for this annotation

GitHub Actions / build

The `then` function is used after `showWarningMessage`, but the function inside `then` is not awaited. This could lead to unexpected behavior as the function might not complete before the next line of code is executed. Consider using `await` with `then` or converting the promise chain to async/await for better readability and error handling. πŸ”„
Copy link

Choose a reason for hiding this comment

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

The then function is used after showWarningMessage, but the function inside then is not awaited. This could lead to unexpected behavior as the function might not complete before the next line of code is executed. Consider using await with then or converting the promise chain to async/await for better readability and error handling. πŸ”„

generated by pr-review-commit async_await

return undefined
}
}
Expand Down
29 changes: 2 additions & 27 deletions packages/vscode/src/vshost.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,17 @@
import * as vscode from "vscode"
import { createVSPath } from "./vspath"
import { TerminalServerManager } from "./servermanager"
import { AzureManager } from "./azuremanager"
import { Uri } from "vscode"
import { ExtensionState } from "./state"
import { Utils } from "vscode-uri"
import { checkFileExists, readFileText } from "./fs"
import { filterGitIgnore } from "../../core/src/gitignore"
import {
parseDefaultsFromEnv,
parseTokenFromEnv,
} from "../../core/src/connection"
import { parseDefaultsFromEnv } from "../../core/src/connection"
import {
DEFAULT_EMBEDDINGS_MODEL,
DEFAULT_MODEL,
DEFAULT_TEMPERATURE,
DOT_ENV_FILENAME,
MODEL_PROVIDER_AZURE,
} from "../../core/src/constants"
import { dotEnvTryParse } from "../../core/src/dotenv"
import {
Expand All @@ -26,15 +21,14 @@
Host,
} from "../../core/src/host"
import { TraceOptions, AbortSignalOptions } from "../../core/src/trace"
import { arrayify, logVerbose, unique } from "../../core/src/util"
import { arrayify, unique } from "../../core/src/util"
import { LanguageModel } from "../../core/src/chat"

export class VSCodeHost extends EventTarget implements Host {
dotEnvPath: string = DOT_ENV_FILENAME
userState: any = {}
readonly path = createVSPath()
readonly server: TerminalServerManager
private _azure: AzureManager
readonly defaultModelOptions = {
model: DEFAULT_MODEL,
temperature: DEFAULT_TEMPERATURE,
Expand All @@ -56,11 +50,6 @@
await parseDefaultsFromEnv(env)
}

get azure() {
if (!this._azure) this._azure = new AzureManager(this.state)
return this._azure
}

get context() {
return this.state.context
}
Expand Down Expand Up @@ -195,23 +184,9 @@
options?: { token?: boolean } & AbortSignalOptions & TraceOptions
): Promise<LanguageModelConfiguration> {
const tok = await this.server.client.getLanguageModelConfiguration(
modelId,
options
)

Check failure on line 189 in packages/vscode/src/vshost.ts

View workflow job for this annotation

GitHub Actions / build

The `askToken` variable is declared but never used. This could lead to confusion for other developers reading the code. Consider removing unused variables to improve code readability. 🧹
Copy link

Choose a reason for hiding this comment

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

The askToken variable is declared but never used. This could lead to confusion for other developers reading the code. Consider removing unused variables to improve code readability. 🧹

generated by pr-review-commit unused_code

const { token: askToken } = options || {}
if (
askToken &&
tok &&
!tok.token &&
tok.provider === MODEL_PROVIDER_AZURE
) {
const azureToken = await this.azure.getOpenAIToken()
if (!azureToken) throw new Error("Azure token not available")
tok.token = "Bearer " + azureToken
tok.curlHeaders = {
Authorization: "Bearer ***",
}
}
return tok
}

Expand Down