rabbah commented on a change in pull request #4941:
URL: https://github.com/apache/openwhisk/pull/4941#discussion_r470994562
##########
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 =>
Review comment:
you could combile the predicates here for example:
```suggestion
case Right(list) if list.nonEmpty && force =>
```
##########
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:
This LGMT but there are some failure modes to consider or at least
document:
- failures to get
- failures to delete
- concurrent modifications
(We don't have a global lock on CRUD ops in namespace.)
So if any of those futures fail, should the future sequence also fail and
report an error?
----------------------------------------------------------------
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]
