bdoyle0182 commented on a change in pull request #4941:
URL: https://github.com/apache/openwhisk/pull/4941#discussion_r473644147
##########
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:
So I'm a little stuck on what the best way to handle the failures of the
get and deletes of individual actions are. Since the future sequence is a part
of the confirm phase of the `deleteEntity` function, if any of them fail; it
will be caught by the `deleteEntity` failure handling. That might be good
enough for example if the `get` fails on one of the actions because it was
concurrently deleted while the request was in flight, it gets a
`DocumentNotFound` exception and will be caught and resolve the api with a
`NotFound` error as expected. The error message will just be entity not found
so not specific as to whether the action failed to delete within it or the
package itself. I don't know what the thoughts are on whether that's
sufficient, I tried to come up with something to combine some code for the
failure handling to reuse but didn't really come up with anything good.
I personally don't think it's a big issue as long as it's documented that
the returned error may not be specific to the package or an action within it,
but curious what you think.
----------------------------------------------------------------
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]
