Page MenuHomePhabricator

Allow Almanac properties to be set and deleted via Conduit
ClosedPublic

Authored by epriestley on Apr 11 2018, 4:28 PM.
Tags
None
Referenced Files
F14402099: D19343.diff
Sun, Dec 22, 11:35 PM
Unknown Object (File)
Tue, Dec 17, 9:51 AM
Unknown Object (File)
Sun, Dec 15, 3:32 AM
Unknown Object (File)
Mon, Dec 9, 6:29 PM
Unknown Object (File)
Mon, Dec 9, 6:29 PM
Unknown Object (File)
Mon, Dec 9, 6:24 PM
Unknown Object (File)
Mon, Dec 9, 5:49 PM
Unknown Object (File)
Thu, Dec 5, 8:09 PM
Subscribers
None

Details

Summary

Depends on D19342. Ref T12414. Ref T13120. This adds an EditEngine extension for editing Almanac properties.

The actual wire format is a little weird. Normally, we'd have a transaction for each property, but since you can pick any property names you want we can't really do that (we'd have to generate infinite transactions).

The transaction wire format anticipates that transactions may eventually get some kind of metadata -- each transaction looks like this:

{
  "type": "title",
  "value": "Example title"
}

...and we can add more keys there. For example, I could have made this transaction look like this:

{
  "type": "property.set",
  "almanac.property.key": "some-key",
  "value": "some-value"
}

However, I don't want to just accept any possible key freely, and it might be a decent chunk of work to formalize this better. It also doesn't feel great.

I just built special transaction types intead, so you:

{
  "type": "property.set",
  "value": {
   "some-key": "some-value",
   ...
  }
}

Internally, we may generate more than one transaction as a result (if the "value" has more than one key).

This feels a bit more natural and is probably easier for clients to use anyway.

Test Plan

Set and deleted Service, Device and Binding properties via the API.

Diff Detail

Repository
rP Phabricator
Branch
almanac25
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 20079
Build 27243: Run Core Tests
Build 27242: arc lint + arc unit

Event Timeline

Is this the first time we need to support basically arbitrary tags on entities? I like this implementation; I'm just wondering if we could use it anywhere else.

This revision is now accepted and ready to land.Apr 11 2018, 4:47 PM

I think in all the vaguely-similar cases we have now we either make the properties their own objects or do something that's basically the same as this, just not quite.

A sort-of example of the former is Phurls, which are technically kind-of arbitrary global keys (i.e., you can't have two ((some-url)) objects with the same name), but each Phurl is a first-class object which is edited separately. Same with Wiki pages, which are sort of key-value-ish.

For stuff like reviewers and subscribers you pass a list like this, but they don't really have values today (more like just a list of keys). Reviewers sort of do have values but there's a little bit of cheating to let @username! and blocking(username) imply a more complex state right now.

Some future stuff, maybe T12799, might look more directly like this.

This revision was automatically updated to reflect the committed changes.