bdoyle0182 commented on a change in pull request #4941:
URL: https://github.com/apache/openwhisk/pull/4941#discussion_r471655208
##########
File path:
core/controller/src/main/scala/org/apache/openwhisk/core/controller/Packages.scala
##########
@@ -121,29 +122,42 @@ trait WhiskPackagesApi extends WhiskCollectionAPI with
ReferencedEntities {
* - 500 Internal Server Error
*/
override def remove(user: Identity, entityName:
FullyQualifiedEntityName)(implicit transid: TransactionId) = {
- deleteEntity(
- WhiskPackage,
- entityStore,
- entityName.toDocId,
- (wp: WhiskPackage) => {
- wp.binding map {
- // this is a binding, it is safe to remove
- _ =>
- Future.successful({})
- } getOrElse {
- // may only delete a package if all its ingredients are deleted
already
- WhiskAction
- .listCollectionInNamespace(entityStore,
wp.namespace.addPath(wp.name), skip = 0, limit = 0) flatMap {
- case Left(list) if (list.size != 0) =>
- Future failed {
- RejectRequest(
- Conflict,
- s"Package not empty (contains ${list.size} ${if (list.size
== 1) "entity" else "entities"})")
- }
- case _ => Future.successful({})
+ parameter('force ? false) { force =>
+ deleteEntity(
+ WhiskPackage,
+ entityStore,
+ entityName.toDocId,
+ (wp: WhiskPackage) => {
+ wp.binding map {
+ // this is a binding, it is safe to remove
+ _ =>
+ Future.successful({})
+ } getOrElse {
+ // may only delete a package if all its ingredients are deleted
already or force flag is set
+ WhiskAction
+ .listCollectionInNamespace(entityStore,
wp.namespace.addPath(wp.name), includeDocs = true, skip = 0, limit = 0) flatMap
{
+ case Right(list) if list.nonEmpty =>
+ if (force) {
+ Future sequence {
+ list.map(action => {
+ WhiskAction.get(entityStore,
wp.fullyQualifiedName(false).add(action.fullyQualifiedName(false).name).toDocId)
flatMap { actionWithRevision =>
Review comment:
Yea good point. From what I can tell with other entity deletions, they
all go through the `deleteEntity` function which will get the document first to
get the revision so I think failure to get is the same as any other deletion.
For failure to delete, some of the list of actions can fail to delete and
the future sequence will fail if a single action fails to delete. And since the
actions are deleted as a part of the confirmation phase of the package delete,
the package won't delete unless they all succeed. I just need to go back and
make sure when an action fails to delete it propagates the correct api failure
or I recover here to transform it to the correct api exception. Should be clear
to the client that if they get a failure for one of the actions failing to
delete, they need to redo the request.
For concurrent modifications I wasn't really thinking about. What's the
consideration I should make there with this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
