Page MenuHomePhabricator

Expose Drydock leases via Conduit
ClosedPublic

Authored by epriestley on Sep 24 2016, 7:19 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Mar 21, 5:59 AM
Unknown Object (File)
Wed, Mar 6, 6:07 AM
Unknown Object (File)
Feb 20 2024, 1:07 PM
Unknown Object (File)
Feb 7 2024, 6:12 AM
Unknown Object (File)
Feb 3 2024, 12:03 PM
Unknown Object (File)
Jan 17 2024, 5:07 AM
Unknown Object (File)
Jan 10 2024, 1:18 PM
Unknown Object (File)
Jan 7 2024, 11:44 PM
Subscribers
Restricted Owners Package

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.

Screen Shot 2018-10-25 at 4.45.22 AM.png (1×2 px, 207 KB)

Diff Detail

Repository
rP Phabricator
Branch
lease1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 21037
Build 28583: Run Core Tests
Build 28582: arc lint + arc unit

Event Timeline

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 edited edge metadata.

Don't yet presume that users will be lease owners

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?

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 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 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
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.