Skip to content

Adding delete all layer versions functionality for layerVersion resource #70

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

Merged

Conversation

Vandita2020
Copy link
Member

Description of changes:
Currently when you delete a LayerVersion it deletes the most recent version of it and keep the previous versions. The functionality we want is to delete all the versions of a LayerVersion when DeleteLayerVersion is executed.

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

@ack-bot ack-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 3, 2023
@ack-bot
Copy link
Collaborator

ack-bot commented Jan 3, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@ack-bot ack-bot requested review from a-hilaly and vijtrip2 January 3, 2023 20:57
@Vandita2020 Vandita2020 force-pushed the layer_version_delete branch from ef13d00 to a7a08e1 Compare January 3, 2023 21:19
Copy link
Member

@a-hilaly a-hilaly left a comment

Choose a reason for hiding this comment

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

Great start the logic is correct! 👍
Left few comments in line

@Vandita2020 Vandita2020 force-pushed the layer_version_delete branch from a7a08e1 to 5115fa4 Compare January 5, 2023 21:20
@Vandita2020
Copy link
Member Author

/test all

@Vandita2020 Vandita2020 force-pushed the layer_version_delete branch from 5115fa4 to f08445f Compare January 5, 2023 23:13
@Vandita2020
Copy link
Member Author

/test all

@Vandita2020 Vandita2020 force-pushed the layer_version_delete branch from f08445f to a432205 Compare January 5, 2023 23:44
@Vandita2020
Copy link
Member Author

/test all

@Vandita2020 Vandita2020 force-pushed the layer_version_delete branch from a432205 to a946469 Compare January 6, 2023 00:11
@Vandita2020
Copy link
Member Author

/test all

@Vandita2020
Copy link
Member Author

/test all

@Vandita2020 Vandita2020 marked this pull request as ready for review January 18, 2023 17:28
@ack-bot ack-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 18, 2023
Copy link
Contributor

@RedbackThomson RedbackThomson left a comment

Choose a reason for hiding this comment

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

Some nits about Go code formatting and syntax

Copy link
Member

@a-hilaly a-hilaly left a comment

Choose a reason for hiding this comment

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

Good job on this @Vandita2020 !
Leaving the lgtm-ing for @RedbackThomson
/approve

@ack-bot
Copy link
Collaborator

ack-bot commented Jan 20, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: A-Hilaly, Vandita2020

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@a-hilaly
Copy link
Member

/test all

1 similar comment
@a-hilaly
Copy link
Member

/test all

Copy link
Contributor

@RedbackThomson RedbackThomson left a comment

Choose a reason for hiding this comment

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

Great yeah I'm happy with this
/lgtm

Comment on lines +25 to +27
// customPreDelete deletes all the previous versions of a
// LayerVersion except the latest version
// This function is used as a sdk_delete hook, to delete all the previous versions of a LayerVersion when delete API call is made
Copy link
Contributor

Choose a reason for hiding this comment

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

tiny nit: These comments seem to be saying the same thing.

@ack-prow ack-prow bot added the lgtm Indicates that a PR is ready to be merged. label Feb 1, 2023
@ack-prow ack-prow bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 1, 2023
@Vandita2020
Copy link
Member Author

/test all

@RedbackThomson
Copy link
Contributor

/lgtm

@ack-prow ack-prow bot added the lgtm Indicates that a PR is ready to be merged. label Feb 1, 2023
@ack-prow
Copy link

ack-prow bot commented Feb 1, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: A-Hilaly, RedbackThomson, Vandita2020

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [A-Hilaly,RedbackThomson]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ack-prow ack-prow bot merged commit a5fa961 into aws-controllers-k8s:main Feb 1, 2023
michaelhtm pushed a commit to rushmash91/lambda-controller that referenced this pull request Feb 5, 2025
…llers-k8s#70)

### Update to ACK runtime `v0.29.2`, code-generator `v0.29.2`

----------

* ACK code-generator `v0.29.2` [release notes](https://github.com/aws-controllers-k8s/code-generator/releases/tag/v0.29.2)
* ACK runtime `v0.29.2` [release notes](https://github.com/aws-controllers-k8s/runtime/releases/tag/v0.29.2)

----------

NOTE:
This PR increments the release version of service controller from `v0.0.29` to `v0.0.30`

Once this PR is merged, release `v0.0.30` will be automatically created for `mq-controller`

**Please close this PR, if you do not want the new patch release for `mq-controller`**

----------

#### stdout for `make build-controller`:

```
building ack-generate ... ok.
==== building mq-controller ====
Copying common custom resource definitions into mq
Building Kubernetes API objects for mq
Generating deepcopy code for mq
Generating custom resource definitions for mq
Building service controller for mq
Generating RBAC manifests for mq
Running gofmt against generated code for mq
Updating additional GitHub repository maintenance files
==== building mq-controller release artifacts ====
Building release artifacts for mq-v0.0.30
Generating common custom resource definitions
Generating custom resource definitions for mq
Generating RBAC manifests for mq
```

----------

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
approved lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants