-
Notifications
You must be signed in to change notification settings - Fork 23
feat(164): specify create overwrites soft deleted #371
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(164): specify create overwrites soft deleted #371
Conversation
odsod
left a comment
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.
@toumorokoshi Huge thanks for taking our feedback into account! From our POV this makes sense. 👍
For the upcoming edition cut I think this clarification is excellent: not too big of a change that solves for the main pain point with an acceptable trade-off. (At least from the POV of our use cases).
And this gives us a longer timeline to explore bigger changes to the soft-delete pattern until the next edition!
gibson042
left a comment
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.
It's been quite a while since I was able to attend a live meeting, but these changes look good to me.
|
I'd like to propose that you can't overwrite a soft-deleted resource unless a "force flag" is included. The flag is a no-op otherwise. |
bfc734c to
2a82b47
Compare
To help ensure that declarative clients do not have a degraded user experience with resources that implement soft-delete, specify that create overwrites an existing soft-deleted resource. This ensures that a declarative tool can still properly apply a resource, overwriting it instead of requiring an out of band force delete or having the user modify the ID of the resource. fixes aep-dev#111 (which includes some significant debate and context on why this approach was chosen).
6e0280a to
21bc0f8
Compare
|
@rambleraptor addressed your feedback! PTAL. |
|
I think this works great. Approved! |
21bc0f8 to
cd90bb4
Compare
To help ensure that declarative clients do not have a degraded user experience with resources that implement soft-delete, specify that create overwrites an existing soft-deleted resource.
This ensures that a declarative tool can still properly apply a resource, overwriting it instead of requiring an out of band force delete or having the user modify the ID of the resource.
fixes #111 (which includes some significant debate and context on why this approach was chosen).
Inlined description:
In #365, there was additional discussion or the viability of the pattern. @odsod brings up valid points that the cost to implement my proposed pattern is quite a bit different (requires two different collections), and perhaps implements a sort of "snapshot" resource collection than a soft-deleted resource, whose main intention is primarily around having a resource that exists until some expiry date rather than a snapshot that should be recovered.
In addition, most who have joined AEP live meetings have raised valid examples of soft deleted resources in their APIs (e.g. cryptographic keys in Azure), and therefore the omissions of this pattern might invalidate those resources.
As such, combined with the primary motivation here to eliminate the poor UX with declarative clients which may need to overwrite the resource, this change acts as a more surgical fix to ensure that a create behaves like any other resource (specifically allowing the recreation of one), even when using the soft delete pattern.
🍱 Types of changes
What types of changes does your code introduce to AEP? Put an
xin the boxesthat apply
📋 Your checklist for this pull request
Please review the AEP Style and Guidance for
contributing to this repository.
General
references AEPs
correctly.
(usually
prettier -w .)./scripts/fix.py💝 Thank you!