Page MenuHomePhabricator

Expose Drydock leases via Conduit
ClosedPublic

Authored by epriestley on Sep 24 2016, 7:19 AM.

Details

Summary

See T13212 for some context and discussion on this being revived.
See T11694 for original context.

Add a query constraint for lease owners and implement the conduit search method for Drydock leases.

Ref T11694. Fixes T13212.

Test Plan
  • Called the API method from conduit and browsed lease queries from the UI.
  • Used the new "ownerPHIDs" constraint via API console.

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

yelirekim updated this revision to Diff 39945.Sep 24 2016, 7:19 AM
yelirekim retitled this revision from to Expose Drydock leases via Conduit.
yelirekim updated this object.
yelirekim edited the test plan for this revision. (Show Details)
yelirekim updated this revision to Diff 39946.Sep 24 2016, 7:28 AM
yelirekim edited edge metadata.

Don't yet presume that users will be lease owners

epriestley added inline comments.Sep 26 2016, 1:39 PM
src/applications/drydock/query/DrydockLeaseSearchEngine.php
56

This isn't a Datasource because owners are a weird mishmash of different object types, I assume? Seems OK to me to let users search by owner, but I think pretty much anything can own a lease right now?

src/applications/drydock/storage/DrydockLease.php
591

What's your use case for reading attributes? I'm not sure we should dump them verbatim -- they may conceivably contain internal data or keys, and I think their structure is somewhat in flux, and they're probably not the most-preferred format for consumption. If possible, clients probably shouldn't be relying on raw attributes.

If this is for completeness, maybe omit it?

If this is for a particular purpose, maybe we can find a better way to expose the data that's more client-consumable?

yelirekim added inline comments.Sep 26 2016, 5:52 PM
src/applications/drydock/query/DrydockLeaseSearchEngine.php
56

Yeah, initially I had this as a user facing typeahead that accepted people as it's datasource, was thinking of combining build steps and people into one datasource, but then you would have to be able to search build steps by name, etc etc.

src/applications/drydock/storage/DrydockLease.php
591

Internally we display like, what version of the workspace you have checked out from the CLI when you're looking at your leases, or the hostname of the machine if you have a machine lease.

We can just implement it as a search attachment when reintegrating with upstream, probably would tend to agree with you more that it needs some work before being exposed over the API.

epriestley commandeered this revision.Oct 25 2018, 11:38 AM
epriestley added a reviewer: yelirekim.

See T13212. I'm going to bring everything except attributes upstream, modulo one or two tiny tweaks.

My version of this is substantially identical, but note that until is now an integer or null and may previously have been a string, possibly including the empty string.

epriestley edited the summary of this revision. (Show Details)Oct 25 2018, 11:44 AM
epriestley edited the test plan for this revision. (Show Details)
  • Remove attributes.
  • Add ownerPHID as an alias.
  • Be a little stricter about until types.
Owners added a subscriber: Restricted Owners Package.Oct 25 2018, 11:46 AM
epriestley edited the summary of this revision. (Show Details)
amckinley accepted this revision.Oct 26 2018, 1:43 AM
This revision is now accepted and ready to land.Oct 26 2018, 1:43 AM
This revision was automatically updated to reflect the committed changes.