-
Notifications
You must be signed in to change notification settings - Fork 0
Implement theia env strategy pattern, using data bridge #151
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
base: main
Are you sure you want to change the base?
Conversation
…heia env strategy
KevinGruber2001
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise looks very good 🚀
src/theia/env-strategy.ts
Outdated
|
|
||
| /** | ||
| * Strategy that polls the data bridge extension for environment variables. | ||
| * Used when SCORPIO_ENV_STRATEGY=data-bridge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the Envrironment Variable is SCORPIO_THEIA_ENV_STRATEGY
src/theia/env-strategy.ts
Outdated
| if (error) { | ||
| reject(error); | ||
| } else { | ||
| resolve(stdout.trim()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Im not very sure about this.
I think resolve returns a empty string for unset ENV variables. Which results in THEIA_FLAG always beeing set to true (since it is empty string and not undefined)
Is this behaviour expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, will fix.
Description
This PR introduces a strategy pattern to load theia environment variables supporting the following strategies:
ProcessEnvStrategy(default): Loads environment variables from the process environment. Functionally equivalent to the strategy used before this PRDataBridgeStrategy(Used if the process environment variable SCORPIO_THEIA_ENV_STRATEGY is 'data-bridge'). Uses the data bridge VSCode extension to load environment variables dynamically at runtime. This allows the scorpio extension to wait until the required environment variables become available externally. This work is important to support scorpio in theia environments that are prewarmed. This strategy uses a separate output channel for observability.Steps for Testing
Testing all possible cases is tricky. Here's how I did it using a helper bash script as seen below.
Relevant scenarios
Helper scripts
A bash script to build extensions locally and mount them into the theia image (change paths accordingly)
Helper script to inject credentials using data-bridge
Review Progress
Screenshots