Dtab delete

[adleong]

• Add a delete method to the DtabStore interface
• Update in-memory and zk stores to implement delete
• Add a DELETE method to the http controller

Fixes #209

[gtcampbell]

idle question only tangentially related to this review: would it be useful to have the various CRUD operations return Future[VersionedDtab] rather than Future[Unit]? That way, the caller knows the state/version of the object as of its creation/update/deletion, rather than having to call a get() later.

[adleong]

There's some discussion of that here: #207. Ideally yes, though the cost to implementing this depends on the backing store.

[gtcampbell]

Got it, thanks for the link. FWIW, k8s also gives us that functionality for free.

[gtcampbell]

👍 this looks good to me, and should be easy to merge into my DtabStore work on #219.

[rmars]

⭐ lgtm!

[olix0r]

Should we actually remove it from the state cache after updating it? Won't it continue to be returned from list() otherwise?

[adleong]

I don't think we want to remove this from the cache. Anyone observing the discarded state var will no longer receive updates (eg if the namespace is recreated). Instead, I fixed list to only list non-empty namespaces.

[gtcampbell]

⭐

[olix0r]

tioli:

I generally prefer to avoid nesting -- easier to understand:

val v = ..
Future.value(v)

might write this as:

def list(): Future[Set[String]] = {
  val keys = dtabStatesMu.synchronized(dtabStates).collect {
    case (k, v) if v.sample.isDefined => k
  }
  Future.value(keys.toSet)
}

[olix0r]

⭐