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

Improve shellEnv #6351

Merged
merged 1 commit into from Oct 7, 2022
Merged

Improve shellEnv #6351

merged 1 commit into from Oct 7, 2022

Conversation

jakolehm
Copy link
Contributor

@jakolehm jakolehm commented Oct 7, 2022

Replaces shell-env npm package with a custom implementation which has similar logic as vscode.

Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
@jakolehm jakolehm added the enhancement New feature or request label Oct 7, 2022
@jakolehm jakolehm requested a review from a team as a code owner October 7, 2022 09:44
@jakolehm jakolehm requested review from ixrock and Nokel81 and removed request for a team October 7, 2022 09:44
* @returns object containing the shell's environment variables. An empty object is
* returned if the call fails.
*/
export async function shellEnv(shell?: string, forceRetry = false) : Promise<EnvironmentVariables> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed forceRetry because it was not used anywhere (dead code is bad).

command = `& '${process.execPath}' -p '''${mark}'' + JSON.stringify(process.env) + ''${mark}'''`;
shellArgs = ["-Login", "-Command"];
} else {
command = `'${process.execPath}' -p '"${mark}" + JSON.stringify(process.env) + "${mark}"'`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically we outsource environment variable parsing to a nodejs process (which is started using shell environment). Markers are needed because shell might output some intro text that would confuse JSON.parse.

@Nokel81 Nokel81 added this to the 6.1.3 milestone Oct 7, 2022
Copy link
Collaborator

@Nokel81 Nokel81 left a comment

Choose a reason for hiding this comment

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

LGTM

@Nokel81 Nokel81 merged commit 388c38f into master Oct 7, 2022
@Nokel81 Nokel81 deleted the improve-shell-env branch October 7, 2022 13:19
@Nokel81 Nokel81 mentioned this pull request Oct 7, 2022
Nokel81 pushed a commit that referenced this pull request Oct 7, 2022
Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>

Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
Signed-off-by: Sebastian Malton <sebastian@malton.name>
@Nokel81 Nokel81 added bug Something isn't working and removed enhancement New feature or request labels Oct 7, 2022
Nokel81 added a commit that referenced this pull request Oct 7, 2022
* Release 6.1.3

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* ignore prerelease tag for kubectl version to download (#6299)

Signed-off-by: Jim Ehrismann <jehrismann@mirantis.com>

Signed-off-by: Jim Ehrismann <jehrismann@mirantis.com>

* Split integration tests into seperate jobs from unit tests for faster CI (#6310)

* Split integration tests into seperate jobs from unit tests for faster CI

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Add logging

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Simplify the matrix

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Remove steps that are part of Makefile

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Fix yml decl

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Switch to using single quotes

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Further clarify the test job names

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Fix invocation

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Attempt to fix traking stdout

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Fix lint

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* And handling for tests failing to start

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Add check for app early exiting

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Add more logging to help with debugging

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Cleanup attemptStart code

Signed-off-by: Sebastian Malton <sebastian@malton.name>

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Fix loading helm release details (#6318)

* Fix loading helm release details

- The helm manifest can sometimes contain KubeJsonApiDataLists
  instead of just KubeJsonApiData entries

- Add additional logging to main for when a route handler throws so that
  we can gain more context in the future

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Update tests

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Fix usage of getHelmReleaseResources

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Add test to verify handling of Lists being returned

Signed-off-by: Sebastian Malton <sebastian@malton.name>

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* improve shellEnv (#6351)

Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>

Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Make creating releases automatic on merge of release PRs (#6353)

* Remove unused bundled-extensions file

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Remove unused release drafter workflow

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Tag releases created using create-release-pr with the release label

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Remove the unneeded tag-release script

Signed-off-by: Sebastian Malton <sebastian@malton.name>

* Add workflow for creating release on the merging of a release PR

Signed-off-by: Sebastian Malton <sebastian@malton.name>

Signed-off-by: Sebastian Malton <sebastian@malton.name>

Signed-off-by: Sebastian Malton <sebastian@malton.name>
Signed-off-by: Jim Ehrismann <jehrismann@mirantis.com>
Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
Co-authored-by: Jim Ehrismann <40840436+jim-docker@users.noreply.github.com>
Co-authored-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants