[vscode] 7/n listeners and .env #1390
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Feb 29, 2024
Ankush-lastmile
force-pushed
the
pr1390
branch
3 times, most recently
from
0a3c8e6
to
8df06ee
Compare
Ankush-lastmile
requested review from
saqadri,
rholinshead,
suyoglastmileai,
jonathanlastmileai and
rossdanlm
as code owners
Ankush-lastmile
commented
Mar 4, 2024
| } | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
for anyone unaware,
[ENV_FILE_PATH] is syntax equivalent to resolving the value of ENV_FILE_PATH as the key for the js object.
rholinshead
reviewed
Mar 4, 2024
vscode-extension/package.json
Outdated
| "vscode-aiconfig.env_file_path": { | ||
| "type": "string", | ||
| "default": "", | ||
| "description": "env_file_path" |
There was a problem hiding this comment.
Description should be clearer on what this is for
There was a problem hiding this comment.
Updated to "File path to your .env file containing your API keys"
added 2 commits
March 4, 2024 19:35
Summary: This diff modifies the AIConfig start and edit commands to take in an dot env path. Due to core utils logic of handling args, the help messages automatically get updated too. Also Removes the loadenv() on server init. This is okay because loadenv is invoked at the run step. [see 4/n](#1384) Test Plan: ``` ankush@ap-mbp aiconfig % aiconfig start -h /opt/homebrew/lib/python3.11/site-packages/pydantic/_internal/_fields.py:151: UserWarning: Field "model_parsers" has conflict with protected namespace "model_". You may be able to resolve this warning by setting `model_config['protected_namespaces'] = ()`. warnings.warn( usage: aiconfig start [-h] [--server-port SERVER_PORT] [--log-level LOG_LEVEL] [--server-mode {debug_servers,debug_backend,prod}] [--parsers-module-path PARSERS_MODULE_PATH] [--env-file-path ENV_FILE_PATH] options: -h, --help show this help message and exit --server-port SERVER_PORT --log-level LOG_LEVEL --server-mode {debug_servers,debug_backend,prod} --parsers-module-path PARSERS_MODULE_PATH **--env-file-path ENV_FILE_PATH** ```
Summary: Stack contains changes that updates overall environment setup flow for vscode extension. This diff adds support to call "SET_ENV_FILE_PATH" implemented in the previous diff. No functional changes. Test Plan:
rholinshead
reviewed
Mar 5, 2024
rholinshead
reviewed
Mar 5, 2024
| context.subscriptions.push(vscode.workspace.onDidChangeConfiguration(async (event) => { | ||
| if (event.affectsConfiguration("vscode-aiconfig")) { | ||
| // Get new env | ||
| const envPath = vscode.workspace.getConfiguration("vscode-aiconfig").get(ENV_FILE_PATH) as string; |
There was a problem hiding this comment.
Same here, unless deleting it makes its value empty string instead of undefined
There was a problem hiding this comment.
Just checked really quickly and this get method returns an empty string when it is never set. I think this might be configured in the package.json but I'm unaware of how this works at the moment.
rholinshead
reviewed
Mar 5, 2024
vscode-extension/src/extension.ts
Outdated
| @@ -208,6 +210,25 @@ export async function activate(context: vscode.ExtensionContext) { | |||
| } | |||
| ) | |||
| ); | |||
|
|
|||
| // Handle changes to the .env path | |||
| context.subscriptions.push(vscode.workspace.onDidChangeConfiguration(async (event) => { | |||
rholinshead
reviewed
Mar 5, 2024
rholinshead
approved these changes
Mar 5, 2024
Summary: When the .env file path is set, vscode will invoke api/set_env on each AIConfig server. Defines a listener to a newly introduced setting for the env_file_path. This ensures that users do not need to restart vscode to pickup env changes. Test Plan: 1. set env file path 2. changes picked up 3. changes picked up on restart 4. Changes picked up when manually removing vscode setting and re-setting env file path and api key https://github.com/lastmile-ai/aiconfig/assets/141073967/07ac1d43-28ef-439f-98e7-94a4e73e11c9
|
Ankush-lastmile
added a commit
that referenced
this pull request
Mar 5, 2024
[vscode] 8/n Remove LCA env checks Summary: Now that a user can specify a .env file path with no restrictions on its location, the LCA checks need to be removed. LCA = Lowest Common Ancestor Test Plan: --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/1398). * __->__ #1398 * #1390 * #1387 * #1389
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[vscode] 7/n listeners and .env
Summary:
When the .env file path is set, vscode will invoke api/set_env on each AIConfig server.
Defines a listener to a newly introduced setting for the env_file_path.
This ensures that users do not need to restart vscode to pickup env changes.
Test Plan:
testplan.mp4
Stack created with Sapling. Best reviewed with ReviewStack.