-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Migrate Prebuild settings β EXP-672 #18842
Changes from all commits
95c458d
4e0d852
8ae49f2
df06300
87047f1
08c4c5a
0d12585
318c52c
8916a4b
df97e8c
e976d40
813b55f
6201906
7e7556d
1fa2565
905d6cc
0e630e8
373ec10
a72a96b
77ba20e
56f41f1
e2eefef
f111f23
63c3c6d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,29 +14,79 @@ export interface ProjectConfig { | |
} | ||
|
||
export interface ProjectSettings { | ||
/** | ||
* Controls settings of prebuilds for this project. | ||
*/ | ||
prebuilds?: PrebuildSettings; | ||
|
||
/** @deprecated see `Project.settings.prebuilds.enabled` instead. */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI, not removing here, as these are required for the migration phase |
||
enablePrebuilds?: boolean; | ||
/** | ||
* Wether prebuilds (if enabled) should only be started on the default branch. | ||
* Defaults to `true` on project creation. | ||
* | ||
* @deprecated see `Project.settings.prebuilds.branchStrategy` instead. | ||
*/ | ||
prebuildDefaultBranchOnly?: boolean; | ||
/** | ||
* Use this pattern to match branch names to run prebuilds on. | ||
* The pattern matching will only be applied if prebuilds are enabled and | ||
* they are not limited to the default branch. | ||
* | ||
* @deprecated see `Project.settings.prebuilds.branchMatchingPattern` instead. | ||
*/ | ||
prebuildBranchPattern?: string; | ||
/** | ||
* how many commits in the commit history a prebuild is good (undefined and 0 means every commit is prebuilt) | ||
* | ||
* @deprecated see `Project.settings.prebuilds.intervall` instead. | ||
*/ | ||
prebuildEveryNthCommit?: number; | ||
|
||
/** | ||
* @deprecated always false | ||
*/ | ||
useIncrementalPrebuilds?: boolean; | ||
|
||
/** | ||
* @deprecated always true (we should kill dangling prebuilds) | ||
*/ | ||
keepOutdatedPrebuildsRunning?: boolean; | ||
// whether new workspaces can start on older prebuilds and incrementally update | ||
/** | ||
* @deprecated always true | ||
*/ | ||
allowUsingPreviousPrebuilds?: boolean; | ||
// how many commits in the commit history a prebuild is good (undefined and 0 means every commit is prebuilt) | ||
prebuildEveryNthCommit?: number; | ||
|
||
// preferred workspace classes | ||
workspaceClasses?: WorkspaceClasses; | ||
} | ||
export namespace ProjectSettings { | ||
export type PrebuildBranchStrategy = "defaultBranch" | "allBranches" | "selectedBranches"; | ||
export namespace PrebuildSettings { | ||
export type BranchStrategy = "default-branch" | "all-branches" | "matched-branches"; | ||
} | ||
|
||
export interface PrebuildSettings { | ||
enable?: boolean; | ||
|
||
/** | ||
* Defines an interval of commits to run new prebuilds for. Defaults to 20 | ||
*/ | ||
prebuildInterval?: number; | ||
|
||
/** | ||
* Which branches to consider to run new prebuilds on. Default to "all-branches" | ||
*/ | ||
branchStrategy?: PrebuildSettings.BranchStrategy; | ||
/** | ||
* If `branchStrategy` s set to "matched-branches", this should define a glob-pattern to be used | ||
* to match the branch to run new prebuilds on. Defaults to "**" | ||
*/ | ||
branchMatchingPattern?: string; | ||
|
||
/** | ||
* Preferred workspace class for prebuilds. | ||
*/ | ||
workspaceClass?: string; | ||
} | ||
|
||
export interface Project { | ||
|
@@ -69,46 +119,33 @@ export namespace Project { | |
return p.name + "-" + p.id; | ||
} | ||
|
||
export type PrebuildSettingsWithDefaults = Required<Pick<PrebuildSettings, "prebuildInterval">> & PrebuildSettings; | ||
|
||
export const PREBUILD_SETTINGS_DEFAULTS: PrebuildSettingsWithDefaults = { | ||
enable: false, | ||
branchMatchingPattern: "**", | ||
prebuildInterval: 20, | ||
branchStrategy: "all-branches", | ||
}; | ||
|
||
/** | ||
* If *no settings* are present on pre-existing projects, this defaults to `true` (enabled) for | ||
* backwards compatibility. This allows to do any explicit migration of data or adjustment of | ||
* the default behavior at a later point in time. | ||
* | ||
* Otherwise this returns the value of the `enablePrebuilds` settings persisted in the given | ||
* project. | ||
* Returns effective prebuild settings for the given project. The resulting settings | ||
* contain default values for properties which are not set explicitly for this project. | ||
*/ | ||
export function isPrebuildsEnabled(project: Project): boolean { | ||
// Defaulting to `true` for backwards compatibility. Ignoring non-boolean for `enablePrebuilds` | ||
// for evaluation here allows to do any explicit migration of data or adjustment of the default | ||
// behavior at a later point in time. | ||
if (!hasPrebuildSettings(project)) { | ||
return true; | ||
} | ||
export function getPrebuildSettings(project: Project): PrebuildSettingsWithDefaults { | ||
// ignoring persisted properties with `undefined` values to exclude them from the override. | ||
const overrides = Object.fromEntries( | ||
Object.entries(project.settings?.prebuilds ?? {}).filter(([_, value]) => value !== undefined), | ||
); | ||
|
||
return !!project.settings?.enablePrebuilds; | ||
return { | ||
...PREBUILD_SETTINGS_DEFAULTS, | ||
...overrides, | ||
}; | ||
} | ||
|
||
export function hasPrebuildSettings(project: Project) { | ||
return !(typeof project.settings?.enablePrebuilds === "undefined"); | ||
} | ||
|
||
export function getPrebuildBranchStrategy(project: Project): ProjectSettings.PrebuildBranchStrategy { | ||
if (!hasPrebuildSettings(project)) { | ||
// returning "all branches" to mimic the default value of projects which were added | ||
// before introduction of persisted settings for prebuilds. | ||
return "allBranches"; | ||
} | ||
if (typeof project.settings?.prebuildDefaultBranchOnly === "undefined") { | ||
return "defaultBranch"; // default value for `settings.prebuildDefaultBranchOnly` | ||
} | ||
if (project.settings?.prebuildDefaultBranchOnly) { | ||
return "defaultBranch"; | ||
} | ||
const prebuildBranchPattern = project.settings?.prebuildBranchPattern?.trim(); | ||
if (typeof prebuildBranchPattern === "string" && prebuildBranchPattern.length > 1) { | ||
return "selectedBranches"; | ||
} | ||
return "allBranches"; | ||
return !(typeof project.settings?.prebuilds === "undefined"); | ||
} | ||
|
||
export interface Overview { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -328,22 +328,22 @@ func setupProjectsService(t *testing.T) (*protocol.MockAPIInterface, v1connect.P | |
|
||
func newProject(p *protocol.Project) *protocol.Project { | ||
r := rand.Int() | ||
b_false := false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] Underscore is an anti-pattern in go. camelCase is the expected |
||
result := &protocol.Project{ | ||
ID: uuid.New().String(), | ||
Name: fmt.Sprintf("team-%d", r), | ||
TeamID: uuid.New().String(), | ||
CloneURL: "https://github.com/easyCZ/foobar", | ||
AppInstallationID: "1337", | ||
Settings: &protocol.ProjectSettings{ | ||
UseIncrementalPrebuilds: true, | ||
UsePersistentVolumeClaim: true, | ||
KeepOutdatedPrebuildsRunning: true, | ||
AllowUsingPreviousPrebuilds: true, | ||
PrebuildEveryNthCommit: 5, | ||
UsePersistentVolumeClaim: true, | ||
WorkspaceClasses: &protocol.WorkspaceClassesSettings{ | ||
Regular: "default", | ||
Prebuild: "default", | ||
}, | ||
PrebuildSettings: &protocol.PrebuildSettings{ | ||
Enable: &b_false, | ||
}, | ||
}, | ||
CreationTime: "2022-09-09T09:09:09.000Z", | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,13 +43,16 @@ message ProjectSettings { | |
} | ||
|
||
message PrebuildSettings { | ||
bool enable_incremental_prebuilds = 1; | ||
bool keep_outdated_prebuilds_running = 2; | ||
bool use_previous_prebuilds = 3; | ||
int32 prebuild_every_nth = 4; | ||
reserved 1; // enable_incremental_prebuilds | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π |
||
reserved 2; // keep_outdated_prebuilds_running | ||
reserved 3; // use_previous_prebuilds | ||
reserved 4; // was prebuild_every_nth | ||
optional bool enable_prebuilds = 5; | ||
optional bool prebuild_default_branch_only = 6; | ||
optional string prebuild_branch_pattern = 7; | ||
reserved 6; // was prebuild_default_branch_only | ||
optional string branch_matching_pattern = 7; | ||
optional string branch_strategy = 8; | ||
optional int32 prebuild_interval = 9; | ||
optional string workspace_class = 10; | ||
} | ||
|
||
message WorkspaceSettings { | ||
|
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.
π