-
Notifications
You must be signed in to change notification settings - Fork 86
fix(scaffold): allow certain template values to be overriden #2130
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,8 +10,9 @@ import { SourceResolver } from "../../../../io"; | |
| import { | ||
| RUNTIME_TEMPLATE_SHORTCUT_NAMES, | ||
| RUNTIME_TEMPLATE_SHORTCUTS, | ||
| ScaffoldRuntimeInputSchema, | ||
| } from "../../types"; | ||
| resolveRuntimeTemplateShortcut, | ||
| } from "../../shortcuts"; | ||
| import { ScaffoldRuntimeInputSchema } from "../../types"; | ||
| import { RuntimeResourceConfigSchema } from "./types"; | ||
|
|
||
| export const createAddRuntimeHandler = (config: AddProjectResourceConfig) => | ||
|
|
@@ -23,7 +24,7 @@ export const createAddRuntimeHandler = (config: AddProjectResourceConfig) => | |
| flag("description", "an optional description of the runtime", z.string().optional()), | ||
| flag( | ||
| "template", | ||
| "a preset of flags to be leveraged in scaffolding the runtime. mutually exclusive with all runtime scaffolding flags", | ||
| "a preset of flags for scaffolding the runtime; compatible flags override preset values", | ||
| z.enum(RUNTIME_TEMPLATE_SHORTCUT_NAMES).optional(), | ||
| ), | ||
| flag("build", "build type: CodeZip or Container", BuildTypeSchema.optional()), | ||
|
|
@@ -108,19 +109,31 @@ export const createAddRuntimeHandler = (config: AddProjectResourceConfig) => | |
| ] as const; | ||
| const presentScaffoldingFlags = scaffoldingFlags.filter((f) => flags[f] !== undefined); | ||
| const isTemplate = flags["template"] !== undefined; | ||
|
|
||
| if (isTemplate && presentScaffoldingFlags.length > 0) | ||
| throw new InputValidationError( | ||
| `--template and --${presentScaffoldingFlags[0]} are mutually exclusive`, | ||
| ); | ||
| const lockedFlag = (["language", "framework"] as const).find( | ||
| (flagName) => flags[flagName] !== undefined, | ||
| ); | ||
| if (isTemplate && lockedFlag) { | ||
| throw new InputValidationError(`--${lockedFlag} cannot override a template`); | ||
| } | ||
|
|
||
| const isCustom = presentScaffoldingFlags.length > 0; | ||
|
|
||
| const source = new SourceResolver({ stdin: config.io.stdin }); | ||
| const apiKey = await source.resolveSecret("api-key", flags["api-key"]); | ||
|
|
||
| const scaffoldRuntimeInput = isTemplate | ||
| ? RUNTIME_TEMPLATE_SHORTCUTS[flags.template!] | ||
| ? resolveRuntimeTemplateShortcut(flags.template!, { | ||
| runtimeName: flags.name, | ||
| ...(flags.build !== undefined && { | ||
| build: flags.build, | ||
| runtimeVersion: flags.build === "CodeZip" ? "PYTHON_3_14" : undefined, | ||
| }), | ||
| ...(flags["model-provider"] !== undefined && { | ||
| modelProvider: flags["model-provider"], | ||
| }), | ||
| ...(apiKey !== undefined && { apiKey }), | ||
| ...(flags.memory !== undefined && { memory: flags.memory }), | ||
| }) | ||
| : isCustom | ||
|
Comment on lines
+112
to
137
Contributor
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. OOS for your PR but would we see any value in making a shared component for the shared functionality b/w create and runtime?
Contributor
Author
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. YES! I'm hoping to come back to this. |
||
| ? parseScaffoldRuntimeInput({ | ||
| runtimeName: flags.name, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,10 +4,9 @@ import { SourceResolver, type AppIO } from "../../../io"; | |
| import { | ||
| RUNTIME_TEMPLATE_SHORTCUT_NAMES, | ||
| RUNTIME_TEMPLATE_SHORTCUTS, | ||
| ScaffoldRuntimeInputSchema, | ||
| type CreateProjectInput, | ||
| type ProjectManager, | ||
| } from "../types"; | ||
| resolveRuntimeTemplateShortcut, | ||
| } from "../shortcuts"; | ||
| import { ScaffoldRuntimeInputSchema, type CreateProjectInput, type ProjectManager } from "../types"; | ||
| import { ProjectNameSchema } from "../../../projectSchemas/project"; | ||
| import { InputValidationError } from "../../../errors"; | ||
|
|
||
|
|
@@ -24,7 +23,7 @@ export const createCreateProjectHandler = (config: CreateProjectHandlerConfig) = | |
| flag("name", "name of the project to create", ProjectNameSchema), | ||
| flag( | ||
| "template", | ||
| "a preset of flags to be leveraged in scaffolding the runtime. mutually exclusive with all runtime scaffolding flags", | ||
| "a preset of flags for scaffolding the runtime; compatible flags override preset values", | ||
| z.enum(RUNTIME_TEMPLATE_SHORTCUT_NAMES).optional(), | ||
| ), | ||
| flag( | ||
|
|
@@ -75,18 +74,33 @@ export const createCreateProjectHandler = (config: CreateProjectHandlerConfig) = | |
|
|
||
| const presentScaffoldingFlags = scaffoldingFlags.filter((f) => flags[f] !== undefined); | ||
| const isTemplate = flags["template"] !== undefined; | ||
| if (presentScaffoldingFlags.length > 0 && isTemplate) | ||
| throw new InputValidationError( | ||
| `--template and --${presentScaffoldingFlags[0]} are mutually exclusive`, | ||
| ); | ||
| const lockedFlag = (["language", "framework"] as const).find( | ||
| (flagName) => flags[flagName] !== undefined, | ||
| ); | ||
| if (isTemplate && lockedFlag) { | ||
| throw new InputValidationError(`--${lockedFlag} cannot override a template`); | ||
| } | ||
|
|
||
| const isCustom = presentScaffoldingFlags.length > 0; | ||
|
|
||
| const source = new SourceResolver({ stdin: config.io.stdin }); | ||
| const apiKey = await source.resolveSecret("api-key", flags["api-key"]); | ||
|
|
||
| const scaffoldRuntimeInput = isTemplate | ||
| ? RUNTIME_TEMPLATE_SHORTCUTS[flags["template"]!] | ||
| ? resolveRuntimeTemplateShortcut(flags["template"]!, { | ||
| ...(flags["runtime-name"] !== undefined && { | ||
| runtimeName: flags["runtime-name"], | ||
| }), | ||
| ...(flags["build"] !== undefined && { | ||
| build: flags["build"], | ||
| runtimeVersion: flags["build"] === "CodeZip" ? "PYTHON_3_14" : undefined, | ||
| }), | ||
| ...(flags["model-provider"] !== undefined && { | ||
| modelProvider: flags["model-provider"], | ||
| }), | ||
| ...(apiKey !== undefined && { apiKey }), | ||
| ...(flags["memory"] !== undefined && { memory: flags["memory"] }), | ||
| }) | ||
|
Comment on lines
+90
to
+103
Contributor
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. Not saying necessarily to change this because it is correct, but this is kind of hard to read at first. To make this more readable we could move the override building logic into The resolver could ignore undefined values and own the
Contributor
Author
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. I like that! I'll have to rebase #2116 on top of this anyway, so I'm going to merge and address this there. |
||
| : isCustom | ||
| ? parseScaffoldRuntimeInput({ | ||
| runtimeName: flags["runtime-name"] ?? flags["name"], | ||
|
|
||
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.
wait why are adding the build override? shouldn't that also belong to the "locked" param?
for eg: strands-python template can be override to container but the generated files won't contain Dockerfile, while the agentcore.json config would reference one.
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.
The build override should work since its passed as a parameter to the template. I haven't wired up container support for the strands-python one yet, so if its not rejecting that's a bug.
Update: it is a bug, let me just fix that here.