-
Notifications
You must be signed in to change notification settings - Fork 512
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
feat(lambda): download serverless land patterns in IDE #6612
base: feature/serverlessland
Are you sure you want to change the base?
Conversation
|
a20965f
to
1e35324
Compare
1e35324
to
e23a824
Compare
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.
Questions in Wizard part, also please add test
packages/core/src/awsService/appBuilder/serverlessLand/wizard.ts
Outdated
Show resolved
Hide resolved
packages/core/src/awsService/appBuilder/serverlessLand/wizard.ts
Outdated
Show resolved
Hide resolved
packages/core/src/awsService/appBuilder/serverlessLand/wizard.ts
Outdated
Show resolved
Hide resolved
}).run() | ||
} | ||
|
||
async function downloadPatternCode(config: CreateServerlessLandWizardForm): Promise<void> { |
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.
Seems you are providing a Official Serverless Land experience here, could you help to update the Download SL logic I have in walkthrough to use your logic here?
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.
If this is going to be used in other places as well, I suggest bringing these function outside of main.py
and into another file in this folder for readability.
if (patterns.length === 0) { | ||
throw new ToolkitError('No patterns found in metadata') | ||
} | ||
let step = 'pattern' |
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.
for line 40, IMO we should avoid public override async run():
when possible. Just put all the async prework out of the wizard. the override async run()
has cause difficulties in testing as my experience.
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 code has been simplified, and the logic that was previously executed within the public override async run()
method has been moved to the public constructor. No longer need to override the run()
function.
e23a824
to
c8e7703
Compare
|
credentials, | ||
defaultRegion, | ||
}).run() | ||
const config = await launchProjectCreationWizard(extContext) | ||
if (!config) { | ||
createResult = 'Cancelled' |
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.
What is the the purpose behind setting value to these two local variables right before returning?
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 two values will be emitted in the telemetry, within the finally
block of the try-catch statement.
}).run() | ||
} | ||
|
||
async function downloadPatternCode(config: CreateServerlessLandWizardForm): Promise<void> { |
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.
If this is going to be used in other places as well, I suggest bringing these function outside of main.py
and into another file in this folder for readability.
}) | ||
const iacOptions = this.metadataManager.getIacOptions(pattern) | ||
if (iacOptions.length === 0) { | ||
throw new Error('No IAC options found for the selected pattern') |
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.
Not a blocker but we should utilize localize()
function instead of hard coding.Example:
localize('AWS.command.launchConfigForm.title', 'Local Invoke and Debug Configuration')
packages/core/src/awsService/appBuilder/serverlessLand/wizard.ts
Outdated
Show resolved
Hide resolved
3e3a123
to
c5104e2
Compare
Problem
This pull request introduces the functionality to allow users to download the selected pattern from the Serverless Land QuickPick flow into a directory of their choice.
Solution
The key changes made are as follows:
metadata.json
file was updated to accommodate the addition of anasset_name
field for each IaC and Runtime pair.metadataManager.ts
file had to be modified to account for these changes in themetadata.json
file.wizard.ts
file. This helps maintain the state of the current QuickPick and directs the user to the previous QuickPick.feature/x
branches will not be squash-merged at release time.