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

fix: resolved id vs name usage and fixed non-unique task names in pipeline. #47

Merged
merged 4 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 30 additions & 19 deletions API.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

82 changes: 55 additions & 27 deletions src/builders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ export const DefaultBuilderOptions: BuilderOptions = {
*/
export class WorkspaceBuilder {
private readonly _logicalID: string;
private _name?: string;
private _binding?: string;
private _description?: string;

/**
Expand All @@ -156,7 +156,7 @@ export class WorkspaceBuilder {
* Gets the name of the workspace.
*/
public get name(): string | undefined {
return this._name;
return this._logicalID;
}

/**
Expand All @@ -167,11 +167,18 @@ export class WorkspaceBuilder {
}

/**
* Sets the name of the workspace.
* @param name
* Gets the binding of task workspace to a pipeline workspace.
*/
public withName(name: string): WorkspaceBuilder {
this._name = name;
public get binding(): string | undefined {
return this._binding;
}

/**
* Sets the binding of a task workspace to a pipeline workspace.
* @param workspace
*/
public withBinding(workspace: string): WorkspaceBuilder {
this._binding = workspace;
return this;
}

Expand Down Expand Up @@ -709,14 +716,14 @@ export class TaskBuilder {
}

/**
* Gets the name of the `Task` built by the `TaskBuilder`.
* Gets the name of the `Task` in the context of a pipeline.
*/
public get name(): string | undefined {
return this._name;
}

/**
* Sets the name of the `Task` being built.
* Sets the name of the `Task` to be used within a pipeline.
* @param name
*/
public withName(name: string): TaskBuilder {
Expand Down Expand Up @@ -833,7 +840,7 @@ export class TaskBuilder {

const props: TaskProps = {
metadata: {
name: this.name,
name: this.logicalID,
labels: this._labels,
annotations: this._annotations,
},
Expand All @@ -857,17 +864,25 @@ export class TaskBuilder {
export class PipelineBuilder {
private readonly _scope: Construct;
private readonly _id: string;
private _name?: string;
private _name: string;
private _description?: string;
private _tasks?: TaskBuilder[];
private _params?: Map<string, ParameterBuilder>;

/**
* Creates a new instance of the `PipelineBuilder` using the given `scope` and
* `id`. `id` also sets the pipeline's name.
* @param scope
* @param id
*/
public constructor(scope: Construct, id: string) {
this._scope = scope;
this._id = id;
this._name = id;
}

/**
* @deprecated pipeline name is set by `id`
* Provides the name for the pipeline task and will be
* rendered as the `name` property.
* @param name
Expand All @@ -881,12 +896,11 @@ export class PipelineBuilder {
* Gets the name of the pipeline
*/
public get name(): string {
return this._name || this._id;
return this._name;
}

/**
* Provides the name for the pipeline task and will be
* rendered as the `name` property.
* Provides the description for the pipeline.
* @param description
*/
public withDescription(description: string): PipelineBuilder {
Expand Down Expand Up @@ -970,14 +984,18 @@ export class PipelineBuilder {
const pipelineWorkspaces = new Map<string, PipelineWorkspace>();
this._tasks?.forEach((t) => {
t.workspaces?.forEach((w) => {
if (w.binding) {
// Only add the workspace on the pipeline level if it is not already
// there...
const ws = pipelineWorkspaces.get(w.name!);
if (!ws) {
pipelineWorkspaces.set(w.name!, {
name: w.name,
description: w.description,
});
const ws = pipelineWorkspaces.get(w.binding);
if (!ws) {
pipelineWorkspaces.set(w.binding, {
name: w.binding,
description: w.description,
});
}
} else {
throw new Error(`Workspace '${w.logicalID}' in Task '${t.name}' has no binding to a workspace in Pipeline '${this.name}'.`);
}
});
});
Expand All @@ -996,9 +1014,19 @@ export class PipelineBuilder {
// the build. Not that it really hurts anything, but it makes the multidoc
// YAML file bigger and more complex than it needs to be.
const taskList: string[] = new Array<string>();
// To ensure that all tasks in the pipeline have unique names.
const taskNames: string[] = new Array<string>();

this._tasks?.forEach((t, i) => {

const taskName = t.name || t.logicalID;
if (taskNames.find(it => {
return it == taskName;
})) {
throw new Error(`Multiple tasks found with name '${taskName}' in Pipeline '${this.name}', but task names must be unique.`);
}
taskNames.push(taskName);

const taskParams: TaskParam[] = new Array<TaskParam>();
const taskWorkspaces: PipelineTaskWorkspace[] = new Array<TaskWorkspace>();

Expand All @@ -1012,11 +1040,11 @@ export class PipelineBuilder {
t.workspaces?.forEach((w) => {
taskWorkspaces.push({
name: w.logicalID,
workspace: w.name,
workspace: w.binding,
});
});

const pt = createOrderedPipelineTask(t, ((i > 0) ? this._tasks![i - 1].logicalID : ''), taskParams, taskWorkspaces);
const pt = createOrderedPipelineTask(t, ((i > 0) ? (this._tasks![i - 1].name || this._tasks![i - 1].logicalID) : ''), taskParams, taskWorkspaces);

pipelineTasks.push(pt);

Expand All @@ -1025,11 +1053,11 @@ export class PipelineBuilder {
// built along with the pipeline, but only if we haven't already
// built the task yet.
if (!taskList.find(it => {
return it == t.name;
return it == t.logicalID;
})) {
t.buildTask();
}
taskList.push(t.name!);
taskList.push(t.logicalID);
}
});

Expand All @@ -1051,19 +1079,19 @@ export class PipelineBuilder {
function createOrderedPipelineTask(t: TaskBuilder, after: string, params: TaskParam[], ws: TaskWorkspace[]): PipelineTask {
if (after) {
return {
name: t.logicalID,
name: t.name || t.logicalID,
taskRef: {
name: t.name,
name: t.logicalID,
},
runAfter: [after],
params: params,
workspaces: ws,
};
}
return {
name: t.logicalID,
name: t.name || t.logicalID,
taskRef: {
name: t.name,
name: t.logicalID,
},
params: params,
workspaces: ws,
Expand Down
Loading
Loading