coder/vscode-coderPublic

NotificationsYou must be signed in to change notification settings
Fork24
Star90

Start workspaces by shelling out to CLI#400

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

code-asher merged 6 commits intocoder:mainfromaaronlehmann:start-with-cli

Dec 6, 2024

Merged

Start workspaces by shelling out to CLI#400

code-asher merged 6 commits intocoder:mainfromaaronlehmann:start-with-cli

Dec 6, 2024

Conversation

Copy link

Contributor

aaronlehmann commentedNov 18, 2024

Replace the REST-API-based start flow with one that shells out to the coder CLI.

Start workspaces by shelling out to CLI

59ac05c

Replace the REST-API-based start flow with one that shells out to thecoder CLI.Signed-off-by: Aaron Lehmann <alehmann@netflix.com>

aaronlehmann force-pushed thestart-with-cli branch fromf8319a8 to59ac05cCompare

November 18, 2024 23:37

matifali requested a review fromcode-asher

November 19, 2024 12:37

code-asher reviewed

Nov 21, 2024

View reviewed changes

Copy link

Member

code-asher left a comment

Choose a reason for hiding this comment

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

Thank you for putting this together! Just to test, I hard-coded--global-config and it worked great, so that is the only thing we really need to figure out.

src/api.ts OutdatedShow resolvedHide resolved

src/api.tsShow resolvedHide resolved

src/api.ts OutdatedShow resolvedHide resolved

src/api.tsShow resolvedHide resolved

Copy link

ContributorAuthor

aaronlehmann commentedNov 21, 2024

Thanks for reviewing! I'm traveling right now but will come back to this in early December.

aaronlehmann added3 commits

December 4, 2024 17:35

Remove unneeded type declaration

cdccb52

Include stderr and full command in workspace start error message

8cdbd3a

Migrate "session_token" file to "session", add --global-config flag t…

437483e

…o "coder start"

code-asher reviewed

Dec 5, 2024

View reviewed changes

Copy link

Member

code-asher left a comment•
edited
Loading

Choose a reason for hiding this comment

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

Looks and works great! The only thing is that it asked me to log in again, which I think is just because of the missingawait, the next line checked forsession but it had not been renamed yet. Subsequent runs are working wonderfully.

src/remote.ts OutdatedShow resolvedHide resolved

src/storage.ts Outdated

Comment on lines 511 to 513

		await fs.stat(oldTokenPath)
		const newTokenPath = this.getSessionTokenPath(label)
		await fs.rename(oldTokenPath, newTokenPath)

Copy link

Member

code-asherDec 5, 2024

Choose a reason for hiding this comment

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

Not a blocker at all, just wanted to mention thatstat is often an anti-pattern; becauserename can also throwENOENT and there is technically no guarantee the file still exists between thestat andrename, it is always necessary to check the error ofrename anyway, so might as well do it with one call.

Copy link

ContributorAuthor

aaronlehmannDec 5, 2024

Choose a reason for hiding this comment

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

Right, that makes sense. Removed thestat.

aaronlehmannand others added2 commits

December 5, 2024 15:39

Add await to migrateSessionToken call

4a03dc5

Co-authored-by: Asher <ash@coder.com>

Avoid unnecessary stat

1230ee5

code-asher approved these changes

Dec 6, 2024

View reviewed changes

Copy link

Member

code-asher left a comment

Choose a reason for hiding this comment

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

Thank you! After merging I will cut a new release.

code-asher merged commitf4eeb36 intocoder:main

Dec 6, 2024

2 checks passed

aaronlehmann added a commit to aaronlehmann/jetbrains-coder that referenced this pull request

Dec 24, 2024

Start workspaces by shelling out to CLI

e7bf4a5

Replace the REST-API-based start flow with one that shells out to thecoder CLI.This is the JetBrains extension equivalent tocoder/vscode-coder#400.

aaronlehmann mentioned this pull request

Dec 24, 2024

Start workspaces by shelling out to CLIcoder/jetbrains-coder#518

Merged

code-asher pushed a commit to coder/jetbrains-coder that referenced this pull request

Jan 3, 2025

Start workspaces by shelling out to CLI (#518)

8a896dd

Replace the REST-API-based start flow with one that shells out to thecoder CLI.This is the JetBrains extension equivalent tocoder/vscode-coder#400.

Labels

None yet

2 participants

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Start workspaces by shelling out to CLI#400

Start workspaces by shelling out to CLI#400

Uh oh!

Conversation

aaronlehmann commentedNov 18, 2024

Uh oh!

code-asher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aaronlehmann commentedNov 21, 2024

Uh oh!

code-asher left a comment•
edited
Loading

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

code-asherDec 5, 2024

Choose a reason for hiding this comment

Uh oh!

aaronlehmannDec 5, 2024

Choose a reason for hiding this comment

Uh oh!

code-asher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Movatterモバイル変換

Start workspaces by shelling out to CLI#400

Start workspaces by shelling out to CLI#400

Uh oh!

Conversation

aaronlehmann commentedNov 18, 2024

Uh oh!

code-asher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aaronlehmann commentedNov 21, 2024

Uh oh!

code-asher left a comment• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

code-asherDec 5, 2024

Choose a reason for hiding this comment

Uh oh!

aaronlehmannDec 5, 2024

Choose a reason for hiding this comment

Uh oh!

code-asher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

code-asher left a comment•
edited
Loading