This implements Conduit calls for querying Phame blogs and Phame posts.
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- Restricted Maniphest Task
- Commits
- Restricted Diffusion Commit
rP24579e295681: Add phame.queryblog and phame.querypost Conduit calls.
Made some calls and they seem to generally work.
Diff Detail
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
- Mark both methods unstable.
- Use after / before instead of offset. With policy-aware queries, performing an offset of 1000 requires loading and filtering at least 1000 objects. We can't just offset in the database, because the first 1000 records might all be invisible to the viewer, and "skip past 1000 records you can see" might mean examining many more records. WIth "after" and "before", skipping records is cheap.
- I think we should move away from defaulting limits to 100, maybe. It feels very arbitrary and liable to burn callers who are making reasonable queries and end up silently discarding results when they get 105 things back some day. With "no limit" as the default, the failure should be obvious (slow queries / too much data), but with "100" as the default, it may not be.
- It looks like we have method name diversity on these things. differential.querydiffs is plural, as is releeph.queryrequests, but Conpherence has singular names. I think we should use plurals and plan to rename the Conpherence methods. The blog query should probaby jsut be phame.query, as a blog is the primary object in the application.
| src/applications/phame/conduit/ConduitAPI_phame_queryblog_Method.php | ||
|---|---|---|
| 47–50 ↗ | (On Diff #16858) | This should probably be withDomains(). Consider fixing the method or omitting this parameter until we can fix it. |