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

feat(aws-lambda): Generates a version when currentVersionOptions is set #23225

Merged
merged 6 commits into from
Dec 5, 2022

Conversation

marciocadev
Copy link
Contributor

Description

Specifying currentVersionOptions for a Function has no effect unless the currentVersion method is called.

Solution

when defining a currentVersionOptions a new version will be created automatically, without the need to execute currentVersion method.

Fixes #23002

All Submissions:

Adding new Construct Runtime Dependencies:

  • This PR adds new construct runtime dependencies following the process described here

New Features

  • Have you added the new feature to an integration test?
    • Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

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 Dec 4, 2022

@aws-cdk-automation aws-cdk-automation requested a review from a team December 4, 2022 15:48
@github-actions github-actions bot added effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 repeat-contributor [Pilot] contributed between 3-5 PRs to the CDK labels Dec 4, 2022
comcalvi
comcalvi previously approved these changes Dec 5, 2022
@mergify mergify bot dismissed comcalvi’s stale review December 5, 2022 20:55

Pull request has been modified.

@comcalvi comcalvi self-assigned this Dec 5, 2022
@mergify
Copy link
Contributor

mergify bot commented Dec 5, 2022

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).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

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

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

@mergify mergify bot merged commit 65eca47 into aws:main Dec 5, 2022
@mergify
Copy link
Contributor

mergify bot commented Dec 5, 2022

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).

@SamStephens
Copy link
Contributor

Thanks @marciocadev

brennanho pushed a commit to brennanho/aws-cdk that referenced this pull request Dec 9, 2022
…et (aws#23225)

## Description
Specifying `currentVersionOptions` for a `Function` has no effect unless the `currentVersion` method is called.

## Solution
when defining a `currentVersionOptions` a new version will be created automatically, without the need to execute `currentVersion` method.

Fixes aws#23002 
----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Construct Runtime Dependencies:

* [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
mergify bot pushed a commit that referenced this pull request Jan 11, 2023
…ion` resource (#23636)

In #23225, we automatically created a `currentVersion` Version object if the `currentVersionOptions` property was specified.

However, this breaks the case where a user both:

* Specifies `currentVersionOptions`
* Never calls `currentVersion` but instead creates a `Version` object directly.

In that case, 2 `Version` resources are created, but the second one fails because Lambda will not allow creating a new Version if nothing changed since the last Version.

Reverting that change.

----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Construct Runtime Dependencies:

* [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
laurelmay added a commit to laurelmay/aws-cdk that referenced this pull request Jan 13, 2023
The AWS CDK uses conventional commits which encourage a scope. In nearly
all cases, contributors omit the `aws-` prefix; though there are some
situations where they're included (such as aws#23552, aws#23225, and others).
Interestingly, it seems that most often the `aws-` prefix is used for
chore/doc commits that don't end up in changelogs anyway.

This actually results in inconsistencies for users in the changelog.
Because the changelog sorts entries alphabetically by scope, changes
that were contributed with `aws-s3` as the scope are listed at the top
of the changelog whereas changes that just used `s3` are sorted further
down. This means it's harder for users to review one (or I suppose 2
with Feature/Fix being separate sections) spots in the changelog to
identify specific modules they care about.

It's also possible that rather than potentially annoying contributors
who have to edit their titles, this should be automatically fixed up in
the changelog generation. The current behavior may also accidentally
encourage users to scan the whole changelog rather than just a few bits
of it.

So it's very understandable if the potential inconvenience from this
change it's worth the minor formatting consistency gain.
laurelmay added a commit to laurelmay/aws-cdk that referenced this pull request Jan 13, 2023
The AWS CDK uses conventional commits which encourage a scope. In nearly
all cases, contributors omit the `aws-` prefix; though there are some
situations where they're included (such as aws#23552, aws#23225, and others).
Interestingly, it seems that most often the `aws-` prefix is used for
chore/doc commits that don't end up in changelogs anyway.

This actually results in inconsistencies for users in the changelog.
Because the changelog sorts entries alphabetically by scope, changes
that were contributed with `aws-s3` as the scope are listed at the top
of the changelog whereas changes that just used `s3` are sorted further
down. This means it's harder for users to review one (or I suppose 2
with Feature/Fix being separate sections) spots in the changelog to
identify specific modules they care about.

It's also possible that rather than potentially annoying contributors
who have to edit their titles, this should be automatically fixed up in
the changelog generation. The current behavior may also accidentally
encourage users to scan the whole changelog rather than just a few bits
of it.

So it's very understandable if the potential inconvenience from this
change it's worth the minor formatting consistency gain.
mergify bot pushed a commit that referenced this pull request Jan 14, 2023
The AWS CDK uses conventional commits which encourage a scope. In nearly
all cases, contributors omit the `aws-` prefix; though there are some
situations where they're included (such as #23552, #23225, and others).
Interestingly, it seems that most often the `aws-` prefix is used for
chore/doc commits that don't end up in changelogs anyway.

This actually results in inconsistencies for users in the changelog.
Because the changelog sorts entries alphabetically by scope, changes
that were contributed with `aws-s3` as the scope are listed at the top
of the changelog whereas changes that just used `s3` are sorted further
down. This means it's harder for users to review one (or I suppose 2
with Feature/Fix being separate sections) spots in the changelog to
identify specific modules they care about.

It's also possible that rather than potentially annoying contributors
who have to edit their titles, this should be automatically fixed up in
the changelog generation. The current behavior may also accidentally
encourage users to scan the whole changelog rather than just a few bits
of it.

So it's very understandable if the potential inconvenience from this
change it's worth the minor formatting consistency gain.


----

### All Submissions:

* [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Construct Runtime Dependencies:

* [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
brennanho pushed a commit to brennanho/aws-cdk that referenced this pull request Jan 20, 2023
…et (aws#23225)

## Description
Specifying `currentVersionOptions` for a `Function` has no effect unless the `currentVersion` method is called.

## Solution
when defining a `currentVersionOptions` a new version will be created automatically, without the need to execute `currentVersion` method.

Fixes aws#23002 
----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Construct Runtime Dependencies:

* [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
brennanho pushed a commit to brennanho/aws-cdk that referenced this pull request Feb 22, 2023
…et (aws#23225)

## Description
Specifying `currentVersionOptions` for a `Function` has no effect unless the `currentVersion` method is called.

## Solution
when defining a `currentVersionOptions` a new version will be created automatically, without the need to execute `currentVersion` method.

Fixes aws#23002 
----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Construct Runtime Dependencies:

* [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*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
effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 repeat-contributor [Pilot] contributed between 3-5 PRs to the CDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(lambda): Remove requirement to call currentVersion in order for currentVersionOptions to take effect
4 participants