Skip to content

Commit

Permalink
chore: reinforce readonly properties (#5957)
Browse files Browse the repository at this point in the history
* chore: declare execution type  as readonly in `TaskNode` and its siblings

* chore: declare concurrency limits as readonly in `BaseTask` and its siblings

* chore: declare task type as readonly in `BaseTask` and its siblings

* chore: rename `cloudbuilder` to camelCased `cloudBuilder`
  • Loading branch information
vvagaytsev committed Apr 25, 2024
1 parent 5fff93d commit 58dee94
Show file tree
Hide file tree
Showing 18 changed files with 43 additions and 43 deletions.
6 changes: 3 additions & 3 deletions core/src/graph/nodes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ export interface TaskRequestParams<T extends Task = Task> extends TaskNodeParams

@Profile()
export class RequestTaskNode<TaskType extends Task = Task> extends TaskNode<TaskType> {
executionType: NodeType = "request"
readonly executionType: NodeType = "request"

override get concurrencyLimit() {
return gardenEnv.GARDEN_HARD_CONCURRENCY_LIMIT
Expand Down Expand Up @@ -267,7 +267,7 @@ export class RequestTaskNode<TaskType extends Task = Task> extends TaskNode<Task

@Profile()
export class ProcessTaskNode<T extends Task = Task> extends TaskNode<T> {
executionType: NodeType = "process"
readonly executionType: NodeType = "process"

override get concurrencyLimit() {
return this.task.executeConcurrencyLimit
Expand Down Expand Up @@ -342,7 +342,7 @@ export class ProcessTaskNode<T extends Task = Task> extends TaskNode<T> {

@Profile()
export class StatusTaskNode<T extends Task = Task> extends TaskNode<T> {
executionType: NodeType = "status"
readonly executionType: NodeType = "status"

override get concurrencyLimit() {
return this.task.statusConcurrencyLimit
Expand Down
8 changes: 4 additions & 4 deletions core/src/plugins/container/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import type { Writable } from "stream"
import type { ActionLog } from "../../logger/log-entry.js"
import type { PluginContext } from "../../plugin-context.js"
import type { SpawnOutput } from "../../util/util.js"
import { cloudbuilder } from "./cloudbuilder.js"
import { cloudBuilder } from "./cloudbuilder.js"
import { styles } from "../../logger/styles.js"

export const validateContainerBuild: BuildActionHandler<"validate", ContainerBuildAction> = async ({ action }) => {
Expand All @@ -43,7 +43,7 @@ export const getContainerBuildStatus: BuildActionHandler<"getStatus", ContainerB
log,
}) => {
// configure concurrency limit for build execute task nodes.
if (await cloudbuilder.isConfiguredAndAvailable(ctx, action)) {
if (await cloudBuilder.isConfiguredAndAvailable(ctx, action)) {
action.executeConcurrencyLimit = CONTAINER_BUILD_CONCURRENCY_LIMIT_CLOUD_BUILDER
} else {
action.executeConcurrencyLimit = CONTAINER_BUILD_CONCURRENCY_LIMIT_LOCAL
Expand Down Expand Up @@ -92,7 +92,7 @@ export const buildContainer: BuildActionHandler<"build", ContainerBuildAction> =
const timeout = action.getConfig("timeout")

let res: SpawnOutput
if (await cloudbuilder.isConfiguredAndAvailable(ctx, action)) {
if (await cloudBuilder.isConfiguredAndAvailable(ctx, action)) {
res = await buildContainerInCloudBuilder({ action, outputStream, timeout, log, ctx })
} else {
res = await buildContainerLocally({
Expand Down Expand Up @@ -186,7 +186,7 @@ async function buildContainerInCloudBuilder(params: {
}
})

const res = await cloudbuilder.withBuilder(params.ctx, params.action, async (builderName) => {
const res = await cloudBuilder.withBuilder(params.ctx, params.action, async (builderName) => {
const extraDockerOpts = ["--builder", builderName]

// we add --push in the Kubernetes local-docker handler when using the Kubernetes plugin with a deploymentRegistry setting.
Expand Down
4 changes: 2 additions & 2 deletions core/src/plugins/container/cloudbuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const cloudBuilderAvailability = new LRUCache<string, CloudBuilderAvailability>(
})

// public API
export const cloudbuilder = {
export const cloudBuilder = {
isConfigured(ctx: PluginContext): boolean {
const { isCloudBuilderEnabled } = getConfiguration(ctx)
if (!isCloudBuilderEnabled) {
Expand All @@ -59,7 +59,7 @@ export const cloudbuilder = {
},

async isConfiguredAndAvailable(ctx: PluginContext, action: Resolved<ContainerBuildAction>) {
if (!cloudbuilder.isConfigured(ctx)) {
if (!cloudBuilder.isConfigured(ctx)) {
return false
}

Expand Down
4 changes: 2 additions & 2 deletions core/src/plugins/kubernetes/container/build/local.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { getManifestInspectArgs } from "./common.js"
import type { ContainerBuildAction } from "../../../container/moduleConfig.js"
import type { BuildActionParams } from "../../../../plugin/action-types.js"
import { k8sGetContainerBuildActionOutputs } from "../handlers.js"
import { cloudbuilder } from "../../../container/cloudbuilder.js"
import { cloudBuilder } from "../../../container/cloudbuilder.js"

export const getLocalBuildStatus: BuildStatusHandler = async (params) => {
const { ctx, action, log } = params
Expand Down Expand Up @@ -74,7 +74,7 @@ export const localBuild: BuildHandler = async (params) => {
const localId = outputs.localImageId
const remoteId = outputs.deploymentImageId

const builtByCloudBuilder = await cloudbuilder.isConfiguredAndAvailable(ctx, action)
const builtByCloudBuilder = await cloudBuilder.isConfiguredAndAvailable(ctx, action)

// TODO: Kubernetes plugin and container plugin are a little bit twisted; Container plugin has some awareness of Kubernetes, but in this
// case it can't detect that the image needs to be pushed when using remote builder, because it can't get the Kubernetes config from ctx.
Expand Down
6 changes: 3 additions & 3 deletions core/src/plugins/kubernetes/container/extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import type {
RunActionExtension,
TestActionExtension,
} from "../../../plugin/action-types.js"
import { cloudbuilder } from "../../container/cloudbuilder.js"
import { cloudBuilder } from "../../container/cloudbuilder.js"
import type {
ContainerBuildAction,
ContainerDeployAction,
Expand Down Expand Up @@ -55,7 +55,7 @@ async function getBuildMode({
ctx: KubernetesPluginContext
action: Resolved<ContainerBuildAction>
}): Promise<ContainerBuildMode> {
if (await cloudbuilder.isConfiguredAndAvailable(ctx, action)) {
if (await cloudBuilder.isConfiguredAndAvailable(ctx, action)) {
// Local build mode knows how to build using Cloud Builder
return "local-docker"
} else {
Expand Down Expand Up @@ -106,7 +106,7 @@ export const k8sContainerBuildExtension = (): BuildActionExtension<ContainerBuil
const provider = ctx.provider as KubernetesProvider

// override build task execute concurrency
if (await cloudbuilder.isConfiguredAndAvailable(ctx, action)) {
if (await cloudBuilder.isConfiguredAndAvailable(ctx, action)) {
action.executeConcurrencyLimit = CONTAINER_BUILD_CONCURRENCY_LIMIT_CLOUD_BUILDER
} else if (provider.config.buildMode === "local-docker") {
action.executeConcurrencyLimit = CONTAINER_BUILD_CONCURRENCY_LIMIT_LOCAL
Expand Down
8 changes: 4 additions & 4 deletions core/src/tasks/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,21 +105,21 @@ interface TaskEvents<O extends ValidResultType> {
}
@Profile()
export abstract class BaseTask<O extends ValidResultType = ValidResultType> extends TypedEventEmitter<TaskEvents<O>> {
abstract type: string
abstract readonly type: string

/**
* How many execute task nodes of this exact type are allowed to run concurrently
*
* Children can override this to set a custom concurrency limit.
*/
abstract executeConcurrencyLimit: number
abstract readonly executeConcurrencyLimit: number

/**
* How many get-status task nodes of this exact type are allowed to run concurrently
*
* Children can override this to set a custom concurrency limit.
*/
abstract statusConcurrencyLimit: number
abstract readonly statusConcurrencyLimit: number

public readonly garden: Garden
public readonly log: Log
Expand Down Expand Up @@ -646,7 +646,7 @@ export abstract class ExecuteActionTask<
return this.action.statusConcurrencyLimit || this.defaultStatusConcurrencyLimit
}

abstract override type: Lowercase<T["kind"]>
abstract override readonly type: Lowercase<T["kind"]>

abstract override getStatus(params: ActionTaskStatusParams<T>): Promise<O & ExecuteActionOutputs<T>>

Expand Down
2 changes: 1 addition & 1 deletion core/src/tasks/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { wrapActiveSpan } from "../util/open-telemetry/spans.js"

@Profile()
export class BuildTask extends ExecuteActionTask<BuildAction, BuildStatus> {
type = "build" as const
readonly type = "build" as const
override defaultStatusConcurrencyLimit = 5
override defaultExecuteConcurrencyLimit = 5
eventName = "buildStatus" as const
Expand Down
6 changes: 3 additions & 3 deletions core/src/tasks/delete-deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ export interface DeleteDeployTaskParams extends BaseActionTaskParams<DeployActio
}

export class DeleteDeployTask extends BaseActionTask<DeployAction, DeployStatus> {
type = "delete-deploy"
override executeConcurrencyLimit = 10
override statusConcurrencyLimit = 10
readonly type = "delete-deploy"
override readonly executeConcurrencyLimit = 10
override readonly statusConcurrencyLimit = 10

dependantsFirst: boolean
deleteDeployNames: string[]
Expand Down
2 changes: 1 addition & 1 deletion core/src/tasks/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function printIngresses(status: DeployStatus, log: ActionLog) {

@Profile()
export class DeployTask extends ExecuteActionTask<DeployAction, DeployStatus> {
type = "deploy" as const
readonly type = "deploy" as const
override defaultStatusConcurrencyLimit = 10
override defaultExecuteConcurrencyLimit = 10

Expand Down
2 changes: 1 addition & 1 deletion core/src/tasks/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ export abstract class PluginActionTask<
T extends Action,
O extends ValidResultType = ValidResultType,
> extends BaseActionTask<T, O> {
type = "plugin"
readonly type = "plugin"
}
6 changes: 3 additions & 3 deletions core/src/tasks/publish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ export interface PublishTaskParams extends BaseActionTaskParams<BuildAction> {
}

export class PublishTask extends BaseActionTask<BuildAction, PublishActionResult> {
type = "publish"
override executeConcurrencyLimit = 5
override statusConcurrencyLimit = 5
readonly type = "publish"
override readonly executeConcurrencyLimit = 5
override readonly statusConcurrencyLimit = 5

/**
* Only defined if --tag option is used in the garden publish command.
Expand Down
6 changes: 3 additions & 3 deletions core/src/tasks/resolve-action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ export interface ResolveActionResults<T extends Action> extends ValidResultType

@Profile()
export class ResolveActionTask<T extends Action> extends BaseActionTask<T, ResolveActionResults<T>> {
type = "resolve-action"
readonly type = "resolve-action"

// TODO: resolving template strings is CPU bound, does single-threaded concurrent execution make it faster or slower?
override executeConcurrencyLimit = 10
override statusConcurrencyLimit = 10
override readonly executeConcurrencyLimit = 10
override readonly statusConcurrencyLimit = 10

getDescription() {
return `resolve ${this.action.longDescription()}`
Expand Down
6 changes: 3 additions & 3 deletions core/src/tasks/resolve-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ const defaultCacheTtl = 3600 // 1 hour
*/
@Profile()
export class ResolveProviderTask extends BaseTask<Provider> {
type = "resolve-provider"
override statusConcurrencyLimit = 20
override executeConcurrencyLimit = 20
readonly type = "resolve-provider"
override readonly statusConcurrencyLimit = 20
override readonly executeConcurrencyLimit = 20

private config: GenericProviderConfig
private plugin: GardenPluginSpec
Expand Down
2 changes: 1 addition & 1 deletion core/src/tasks/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class RunFailedError extends GardenError {

@Profile()
export class RunTask extends ExecuteActionTask<RunAction, GetRunResult> {
type = "run" as const
readonly type = "run" as const

getDescription() {
return this.action.longDescription()
Expand Down
2 changes: 1 addition & 1 deletion core/src/tasks/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export interface TestTaskParams extends BaseActionTaskParams<TestAction> {

@Profile()
export class TestTask extends ExecuteActionTask<TestAction, GetTestResult> {
type = "test" as const
readonly type = "test" as const

silent: boolean

Expand Down
6 changes: 3 additions & 3 deletions core/test/unit/src/graph/solver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ interface TestTaskResult extends ValidResultType {
// TODO-G2: Implement equivalent test cases for the new graph

export class TestTask extends BaseTask<TestTaskResult> {
override statusConcurrencyLimit = 10
override executeConcurrencyLimit = 10
override readonly statusConcurrencyLimit = 10
override readonly executeConcurrencyLimit = 10

type = "test"
readonly type = "test"

name: string
state: ActionState
Expand Down
6 changes: 3 additions & 3 deletions core/test/unit/src/tasks/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ describe("BaseActionTask", () => {
const projectRoot = getDataDir("test-project-test-deps")

class TestTask extends BaseActionTask<TestAction, ValidResultType> {
override statusConcurrencyLimit = 10
override executeConcurrencyLimit = 10
override readonly statusConcurrencyLimit = 10
override readonly executeConcurrencyLimit = 10

type = "test"
readonly type = "test"

getDescription() {
return "foo"
Expand Down
4 changes: 2 additions & 2 deletions plugins/pulumi/src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,8 @@ export type PulumiCommandResult = ValidResultType

@Profile()
class PulumiPluginCommandTask extends PluginActionTask<PulumiDeploy, PulumiCommandResult> {
override statusConcurrencyLimit: number
override executeConcurrencyLimit: number
override readonly statusConcurrencyLimit: number
override readonly executeConcurrencyLimit: number

pulumiParams: PulumiBaseParams
commandName: string
Expand Down

0 comments on commit 58dee94

Please sign in to comment.