[server] Infer config for non-configured repositories #7383

svenefftinge · 2021-12-29T12:47:20Z

This change will use the configuration inference for all workspaces as the fallback.

Testing

Start a workspace on a repository without a .gitpod.yml and find a derived .gitpod.yml in the opened workspace.

Release Notes

Automatically propose a configuration for non-configured repositories.

Fixes #6921

svenefftinge · 2021-12-29T14:32:14Z

/werft run

👍 started the job as gitpod-build-se-init.4

svenefftinge · 2021-12-29T14:36:02Z

/werft run

👍 started the job as gitpod-build-se-init.5

svenefftinge · 2021-12-29T14:44:05Z

/werft run

👍 started the job as gitpod-build-se-init.6

svenefftinge · 2021-12-29T16:23:55Z

/werft run

👍 started the job as gitpod-build-se-init.8

gtsiolis

UX works like a charm and definitely improves the onboarding experience for non-configured repos. 🔮

Left one minor comment (issue) which could be tackled separately. ❗

🍊 🍊 🍊 🍊

Wondering if there’s anything if could do to improve feature (auto-detection and file generation) visibility here and make it less surprising to users when they find out about the new untracked .gitpod.yml file. Probably fine to do also out of the scope of this PR in the spirit of MVC (minimum viable change). ➿

For example, we could:

Add an additional comment in the generated .gtipod.yml to describe what this file is and how to change it by linking to relevant docs.
Trigger (yet) another notification inside the editor to inform the user about project detection with an action to open the .gitpod.yml.
Inform the user about project type detection during workspace loading (workspace start page) for authorized users and during login for anonymous users.

Successful project detection is something we could also surface when the project is added, in #7271. 💡 Cc @jldec

🍋 🍋 🍋 🍋

Approving to unblock merging but holding in case someone from the team could take a closer look at the code changes. 👀

/hold

gtsiolis · 2021-12-29T21:28:12Z

components/server/src/workspace/config-provider.ts

                }
            }

            if (!customConfig) {
                return undefined;
            }

-            return { customConfig, configBasePath };
+            return { customConfig, configBasePath, literalConfig: { '.gitpod.yml': customConfigString || ''} };


issue(non-blocking): Probably not the best line to add this comment (😅) but is it expected that the new untracked auto-generated .gitpod.yml is not visible as changelist in the workspaces list when the workspace stops?

Workspaces with new auto-generated .gitpod.yml and one file change

This is pretty unrelated and should work as reliable as it does with other pending changes.
Related issue #7370

roboquat · 2021-12-29T21:29:53Z

LGTM label has been added.

Git tree hash: 6558a64c319c3a6cd6f31611246262722344d43a

svenefftinge · 2021-12-30T08:28:11Z

Add an additional comment in the generated .gtipod.yml

That makes a ton of sense. We could also build a VS Code extension for .gitpod.yml files that sports code lenses and easy access to documentation. Adding a comment if definitely the lower-hanging thing to do. I'd love to do this on a separate PR, though.

jankeromnes

As mentioned on Slack, wow, this is epic! 🎸

Super clean implementation ✨ the code looks good to me, and works as advertised (I've even tested with an empty repository). 🔥🔥🔥

Bonus points for cleaning up the old code 🧹 (side note: I think we might also want to delete the language detection code in the future).

Also, I think a cool follow-up could be to add a comment to the generated .gitpod.yml file (e.g. to briefly explain what this file is / why it's there).

/approve

jankeromnes · 2021-12-30T13:10:39Z

components/server/src/config/configuration-service.ts

+        if (typeof contextURLOrContext === 'string') {
+            const normalizedContextUrl = this.contextParser.normalizeContextURL(contextURLOrContext);
+            commitContext = (await this.contextParser.handle(ctx, user, normalizedContextUrl)) as CommitContext;
+        } else {
+            commitContext = contextURLOrContext;
+        }


jankeromnes · 2021-12-30T13:10:54Z

components/server/package.json

@@ -57,7 +57,6 @@
    "express-mysql-session": "^2.1.0",
    "express-session": "^1.15.6",
    "fs-extra": "^10.0.0",
-    "gitpod-yml-inferrer": "^1.2.0",


jankeromnes · 2021-12-30T13:21:53Z

components/server/src/workspace/config-inference-provider.ts

-/**
- * Copyright (c) 2020 Gitpod GmbH. All rights reserved.
- * Licensed under the GNU Affero General Public License (AGPL).
- * See License-AGPL.txt in the project root for license information.
- */
-
-import { inject, injectable, postConstruct } from "inversify";
-import { CommitContext, User, WorkspaceConfig } from "@gitpod/gitpod-protocol";
-import { HostContextProvider } from "../auth/host-context-provider";
-import { Config } from "../config";
-
-export type LanguageConfigProvider = (language: string, user: User, commit: CommitContext) => Promise<WorkspaceConfig | undefined>;
-
-@injectable()
-export class ConfigInferenceProvider {
-    @inject(HostContextProvider) protected readonly hostContextProvider: HostContextProvider;
-    @inject(Config) protected readonly config: Config;
-
-    protected readonly languageConfigProvider = new Map<string, LanguageConfigProvider>();
-
-    @postConstruct()
-    public registerLanguageInferrer() {
-        this.languageConfigProvider.set('Go', this.computeGoConfig.bind(this));
-    }
-
-    public async infer(user: User, commit: CommitContext): Promise<WorkspaceConfig | undefined> {
-        const host = commit.repository.host;
-        const hostContext = this.hostContextProvider.get(host);
-        if (!hostContext || !hostContext.services) {
-            return undefined;
-        }
-        const repoHost = hostContext.services;
-        const languages = await repoHost.languagesProvider.getLanguages(commit.repository, user);
-        const topLanguage = this.findTopLanguage(languages);
-        if (topLanguage === undefined) {
-            return undefined;
-        }
-
-        const configProvider = this.languageConfigProvider.get(topLanguage);
-        if (configProvider === undefined) {
-            return undefined;
-        }
-
-        return configProvider(topLanguage, user, commit);
-    }
-
-    protected findTopLanguage(languages: any) {
-        let result: string | undefined;
-        let topScore: number | undefined;
-        Object.keys(languages).forEach(lang => {
-            let score = languages[lang];
-            if (topScore === undefined || score > topScore) {
-                result = lang;
-                topScore = score;
-            }
-        });
-        return result;
-    }
-
-    protected async computeGoConfig(lang: String, user: User, commit: CommitContext): Promise<WorkspaceConfig | undefined> {
-        return {
-            ports: [],
-            tasks: [
-                {
-                    init: `test -f go.mod && go get -v ./...`
-                }
-            ],
-            image: this.config.workspaceDefaults.workspaceImage,
-        } as WorkspaceConfig;
-    }
-
-}


jankeromnes · 2021-12-30T13:23:00Z

components/server/src/workspace/config-provider.ts

-                let customConfigString = await contextRepoConfig;
-                let fromProjectDB = false;
+                customConfigString = await contextRepoConfig;
+                let origin: WorkspaceConfig["_origin"] = 'repo';


jankeromnes · 2021-12-30T13:27:37Z

components/server/src/workspace/config-provider.ts

+                if (!customConfigString) {
+                    // if there's still no configuration, let's infer one
+                    customConfigString = await inferredConfig;
+                    origin = "derived";


Super-nit: Technically, I think this should be conditional on the inferrer returning something (however, in the current state of the code, this doesn't matter because of the if (!customConfig) { return undefined; } below).

Suggested change

origin = "derived";

if (customConfigString) {

origin = "derived";

}

jankeromnes · 2021-12-30T13:28:55Z

components/server/src/workspace/workspace-factory.ts

+            if (config._origin === 'derived' && literalConfig) {
+                (context as any as AdditionalContentContext).additionalFiles = { ... literalConfig };
+            }


roboquat · 2021-12-30T13:39:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gtsiolis, jankeromnes

Associated issue: #6921

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [gtsiolis,jankeromnes]
~~components/server/OWNERS~~ [gtsiolis,jankeromnes]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

svenefftinge · 2021-12-30T13:45:26Z

/unhold

removed unused dependency

2414c9a

roboquat added do-not-merge/work-in-progress do-not-merge/release-note-label-needed team: webapp Issue belongs to the WebApp team size/XS size/XL release-note and removed size/XS do-not-merge/release-note-label-needed labels Dec 29, 2021

[server] derive configuration when no explicit one

5b76ac5

Fixes #6921

svenefftinge force-pushed the se/init branch from bead804 to 5b76ac5 Compare December 29, 2021 14:25

svenefftinge changed the title ~~[WIP] infer config~~ [server] Infer config for non-configured repositories Dec 29, 2021

roboquat removed the do-not-merge/work-in-progress label Dec 29, 2021

gtsiolis approved these changes Dec 29, 2021

View reviewed changes

roboquat added the do-not-merge/hold label Dec 29, 2021

roboquat assigned gtsiolis Dec 29, 2021

roboquat added the lgtm label Dec 29, 2021

roboquat added the approved label Dec 29, 2021

jankeromnes approved these changes Dec 30, 2021

View reviewed changes

roboquat assigned jankeromnes Dec 30, 2021

roboquat removed the do-not-merge/hold label Dec 30, 2021

roboquat merged commit 91c09f7 into main Dec 30, 2021

roboquat deleted the se/init branch December 30, 2021 13:45

svenefftinge mentioned this pull request Dec 30, 2021

[server] infer extensions and add a comment #7392

Merged

roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Jan 4, 2022

jldec mentioned this pull request Feb 27, 2022

Epic: Deprecate definitely-gp #8473

Open

2 tasks

jldec mentioned this pull request May 13, 2022

Deprecate gitpod.yml config stored in the DB #9102

Closed

AlexTugarev mentioned this pull request May 20, 2022

Instructions from .gitpod.yml are build run while it does not exist #9505

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[server] Infer config for non-configured repositories #7383

[server] Infer config for non-configured repositories #7383

svenefftinge commented Dec 29, 2021 •

edited

Loading

svenefftinge commented Dec 29, 2021 •

edited by werft-gitpod-dev-com bot

Loading

svenefftinge commented Dec 29, 2021 •

edited by werft-gitpod-dev-com bot

Loading

svenefftinge commented Dec 29, 2021 •

edited by werft-gitpod-dev-com bot

Loading

svenefftinge commented Dec 29, 2021 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis left a comment •

edited

Loading

gtsiolis Dec 29, 2021 •

edited

Loading

svenefftinge Dec 30, 2021 •

edited

Loading

roboquat commented Dec 29, 2021

svenefftinge commented Dec 30, 2021

jankeromnes left a comment •

edited

Loading

jankeromnes Dec 30, 2021

jankeromnes Dec 30, 2021

jankeromnes Dec 30, 2021

jankeromnes Dec 30, 2021

jankeromnes Dec 30, 2021

jankeromnes Dec 30, 2021

roboquat commented Dec 30, 2021

svenefftinge commented Dec 30, 2021

[server] Infer config for non-configured repositories #7383

[server] Infer config for non-configured repositories #7383

Conversation

svenefftinge commented Dec 29, 2021 • edited Loading

Testing

Release Notes

svenefftinge commented Dec 29, 2021 • edited by werft-gitpod-dev-com bot Loading

svenefftinge commented Dec 29, 2021 • edited by werft-gitpod-dev-com bot Loading

svenefftinge commented Dec 29, 2021 • edited by werft-gitpod-dev-com bot Loading

svenefftinge commented Dec 29, 2021 • edited by werft-gitpod-dev-com bot Loading

gtsiolis left a comment • edited Loading

Choose a reason for hiding this comment

gtsiolis Dec 29, 2021 • edited Loading

Choose a reason for hiding this comment

svenefftinge Dec 30, 2021 • edited Loading

Choose a reason for hiding this comment

roboquat commented Dec 29, 2021

svenefftinge commented Dec 30, 2021

jankeromnes left a comment • edited Loading

Choose a reason for hiding this comment

jankeromnes Dec 30, 2021

Choose a reason for hiding this comment

jankeromnes Dec 30, 2021

Choose a reason for hiding this comment

jankeromnes Dec 30, 2021

Choose a reason for hiding this comment

jankeromnes Dec 30, 2021

Choose a reason for hiding this comment

jankeromnes Dec 30, 2021

Choose a reason for hiding this comment

jankeromnes Dec 30, 2021

Choose a reason for hiding this comment

roboquat commented Dec 30, 2021

svenefftinge commented Dec 30, 2021

svenefftinge commented Dec 29, 2021 •

edited

Loading

svenefftinge commented Dec 29, 2021 •

edited by werft-gitpod-dev-com bot

Loading

svenefftinge commented Dec 29, 2021 •

edited by werft-gitpod-dev-com bot

Loading

svenefftinge commented Dec 29, 2021 •

edited by werft-gitpod-dev-com bot

Loading

svenefftinge commented Dec 29, 2021 •

edited by werft-gitpod-dev-com bot

Loading

gtsiolis left a comment •

edited

Loading

gtsiolis Dec 29, 2021 •

edited

Loading

svenefftinge Dec 30, 2021 •

edited

Loading

jankeromnes left a comment •

edited

Loading