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

chore(lambda): add toString to lambda architecture #26145

Merged
merged 6 commits into from
Jul 17, 2023

Conversation

lukey-aleios
Copy link
Contributor

Add toString to Lambda Architecture property

When throwing errors during synthesis, you can't see the architecture chosen for a lambda. Adding this toString method solves that

Closes #26117


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@gitpod-io
Copy link

gitpod-io bot commented Jun 28, 2023

@github-actions github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Jun 28, 2023
@aws-cdk-automation aws-cdk-automation requested a review from a team June 28, 2023 13:30
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

@lukey-aleios lukey-aleios changed the title chore(aws-lambda): add toString to lambda architecture chore(lambda): add toString to lambda architecture Jun 28, 2023
@aws-cdk-automation aws-cdk-automation dismissed their stale review June 28, 2023 13:36

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jun 28, 2023
Copy link
Contributor

@corymhall corymhall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a unit test for this?

@corymhall corymhall removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jun 30, 2023
@lukey-aleios lukey-aleios force-pushed the lambda-architecture-tostring branch from 5f4a004 to 46261ac Compare June 30, 2023 13:57
@mergify mergify bot dismissed corymhall’s stale review June 30, 2023 13:57

Pull request has been modified.

@lukey-aleios lukey-aleios force-pushed the lambda-architecture-tostring branch 3 times, most recently from 60c1b9f to e5b29be Compare June 30, 2023 15:05
@lukey-aleios lukey-aleios force-pushed the lambda-architecture-tostring branch from e5b29be to 00b9c84 Compare June 30, 2023 15:10
@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jun 30, 2023
@lukey-aleios lukey-aleios reopened this Jun 30, 2023
@lukey-aleios lukey-aleios force-pushed the lambda-architecture-tostring branch from 7fcce5a to 3aa874b Compare June 30, 2023 15:20
Copy link
Contributor

@mrgrain mrgrain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lukey-aleios How are you doing with the unit test? If you need any help checkout the Community Slack on cdk.dev channel #contributing

@mrgrain mrgrain removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jul 14, 2023
@lukey-aleios
Copy link
Contributor Author

@lukey-aleios How are you doing with the unit test? If you need any help checkout the Community Slack on cdk.dev channel #contributing

Hey! I've been away for the past two weeks so will be able to get to this early next week 😄

@mrgrain
Copy link
Contributor

mrgrain commented Jul 14, 2023

Hey! I've been away for the past two weeks so will be able to get to this early next week 😄

Awesome, thank you!

@mrgrain
Copy link
Contributor

mrgrain commented Jul 14, 2023

If the automation closes your PR, please just open a new one once you have had time to address any issues. Pro tip: Open the PR in Draft mode and our automation won't touch it.

@mergify mergify bot dismissed mrgrain’s stale review July 15, 2023 15:49

Pull request has been modified.

@lukey-aleios lukey-aleios marked this pull request as draft July 15, 2023 15:51
@lukey-aleios lukey-aleios marked this pull request as ready for review July 15, 2023 16:27
@mrgrain
Copy link
Contributor

mrgrain commented Jul 17, 2023

Your test case doesn't quite work out

aws-cdk-lib: FAIL aws-lambda/test/architecture.test.js
aws-cdk-lib:   � architecture › toString to return the architecture name
aws-cdk-lib:     expect(received).toEqual(expected) // deep equality
aws-cdk-lib:     Expected: "x86_64"
aws-cdk-lib:     Received: {"dockerPlatform": "linux/amd64", "name": "x86_64"}
aws-cdk-lib:     �[0m �[90m 17 |�[39m�[0m
aws-cdk-lib:     �[0m �[90m 18 |�[39m     �[90m// THEN�[39m�[0m
aws-cdk-lib:     �[0m�[31m�[1m>�[22m�[39m�[90m 19 |�[39m     expect(testLambda�[33m.�[39marchitecture)�[33m.�[39mtoEqual(testLambda�[33m.�[39marchitecture�[33m.�[39mname)�[33m;�[39m�[0m
aws-cdk-lib:     �[0m �[90m    |�[39m                                     �[31m�[1m^�[22m�[39m�[0m
aws-cdk-lib:     �[0m �[90m 20 |�[39m   })�[33m;�[39m�[0m
aws-cdk-lib:     �[0m �[90m 21 |�[39m })�[33m;�[39m�[0m
aws-cdk-lib:     �[0m �[90m 22 |�[39m�[0m

@lukey-aleios lukey-aleios force-pushed the lambda-architecture-tostring branch from f2a595b to 0910e55 Compare July 17, 2023 10:11
@lukey-aleios
Copy link
Contributor Author

Your test case doesn't quite work out

aws-cdk-lib: FAIL aws-lambda/test/architecture.test.js
aws-cdk-lib:   � architecture › toString to return the architecture name
aws-cdk-lib:     expect(received).toEqual(expected) // deep equality
aws-cdk-lib:     Expected: "x86_64"
aws-cdk-lib:     Received: {"dockerPlatform": "linux/amd64", "name": "x86_64"}
aws-cdk-lib:     �[0m �[90m 17 |�[39m�[0m
aws-cdk-lib:     �[0m �[90m 18 |�[39m     �[90m// THEN�[39m�[0m
aws-cdk-lib:     �[0m�[31m�[1m>�[22m�[39m�[90m 19 |�[39m     expect(testLambda�[33m.�[39marchitecture)�[33m.�[39mtoEqual(testLambda�[33m.�[39marchitecture�[33m.�[39mname)�[33m;�[39m�[0m
aws-cdk-lib:     �[0m �[90m    |�[39m                                     �[31m�[1m^�[22m�[39m�[0m
aws-cdk-lib:     �[0m �[90m 20 |�[39m   })�[33m;�[39m�[0m
aws-cdk-lib:     �[0m �[90m 21 |�[39m })�[33m;�[39m�[0m
aws-cdk-lib:     �[0m �[90m 22 |�[39m�[0m

Should be fixed now!

@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jul 17, 2023
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 2782aaa
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Contributor

mergify bot commented Jul 17, 2023

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 3b9c983 into aws:main Jul 17, 2023
@lukey-aleios lukey-aleios deleted the lambda-architecture-tostring branch July 17, 2023 13:18
colifran pushed a commit that referenced this pull request Jul 17, 2023
Add  toString to Lambda Architecture property

When throwing errors during synthesis, you can't see the architecture chosen for a lambda. Adding this toString method solves that

Closes #26117

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
bmoffatt pushed a commit to bmoffatt/aws-cdk that referenced this pull request Jul 29, 2023
Add  toString to Lambda Architecture property

When throwing errors during synthesis, you can't see the architecture chosen for a lambda. Adding this toString method solves that

Closes aws#26117

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lambda: Updating The Lambda Architecture Properties with toString() methods
4 participants