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

code server web: have a commit in the static and callback routes #147311

Merged
merged 5 commits into from May 3, 2022

Conversation

aeschli
Copy link
Contributor

@aeschli aeschli commented Apr 12, 2022

The bult-in web server used for code server web accesses sources and artifacts using the /static and /callback routes.

The pr adds a quality-commit segment in from of these.

That helps with caching and allows to hand over from one server (running an older version) to another server, running the latest version.

  • update expiry date
  • websoccket
  • /vscode-remote-resource

@aeschli aeschli self-assigned this Apr 12, 2022
@alexdima alexdima self-requested a review April 21, 2022 11:07
@alexdima alexdima self-assigned this Apr 21, 2022
src/vs/code/browser/workbench/workbench-dev.html Outdated Show resolved Hide resolved
src/vs/code/browser/workbench/workbench.html Outdated Show resolved Hide resolved
src/vs/server/node/webClientServer.ts Outdated Show resolved Hide resolved
src/vs/server/node/webClientServer.ts Outdated Show resolved Hide resolved
src/vs/server/node/webClientServer.ts Outdated Show resolved Hide resolved
src/vs/server/node/webClientServer.ts Show resolved Hide resolved
@@ -148,7 +159,7 @@ export class WebClientServer {
return serveError(req, res, 400, `Bad request.`);
}

return serveFile(this._logService, req, res, filePath, headers);
return serveFile(filePath, this._environmentService.isBuilt ? CacheControl.NO_EXPIRY : CacheControl.NO_CACHING, this._logService, req, res, headers);
Copy link
Member

Choose a reason for hiding this comment

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

When running from source, this should be CacheControl.ETAG to maintain existing behaviour.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When running from sources I prefer no caching and I don't see a benefit of sticking to ETags. So unless you strongly object, I'd make that change.

Copy link
Member

Choose a reason for hiding this comment

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

Let's continue in #148642

src/vs/server/node/webClientServer.ts Outdated Show resolved Hide resolved
@aeschli aeschli merged commit 4f939fc into main May 3, 2022
@aeschli aeschli deleted the aeschli/commitInRoute branch May 3, 2022 14:15
@aeschli
Copy link
Contributor Author

aeschli commented May 3, 2022

Merged the web server changes. I'll create a new PR for the remaining changes.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants