Page MenuHomePhabricator
Create Task
Maniphest T11439

Retrieve Diff PHID via phid.lookup
Closed, ResolvedPublic
Actions

Description

The root problem is that I want to update the build status for a diff that has been pushed to a staging area. What I have is the Diff ID. What I need is the PHID of the associated harbormaster build. I believe that if I can get the PHID of the Diff, I can use harbormaster.querybuildables to get the PHID of the Build, and harbormaster.sendmessage to update the Build status. The problem I am having is that I cannot get the PHID for the diff.

phid.lookup can be used to get the PHID of Revisions by name (D123), but not Diffs (Diff 1234). I am able to get both by their respective PHIDs. It would be very useful to also be able to get the PHID for a diff if all I have is the Diff Id.

Revisions and Commits

rP Phabricator
Needs ReviewD16376 Add support for Differential Diffs to phid.lookup / PhabricatorObjectQuery

Related Objects

Event Timeline

dpkp created this task.Aug 7 2016, 1:38 AM
epriestley closed this task as Resolved.Aug 7 2016, 12:54 PM
epriestley claimed this task.
epriestley added a subscriber: epriestley.

Try differential.querydiffs.

dpkp added a comment.Aug 7 2016, 5:56 PM

I dont see PHID in the results of differential.querydiffs ?

dpkp added a revision: D16376: Add support for Differential Diffs to phid.lookup / PhabricatorObjectQuery.Aug 7 2016, 5:59 PM
epriestley reopened this task as Open.Aug 7 2016, 7:02 PM
epriestley added a comment.Aug 8 2016, 3:56 PM

Can you walk me through your workflow in more detail? The expectation is:

  • Harbormaster triggers a build.
  • As part of your build plan, you send whatever is doing the build the build target PHID using the target.phid variable.
  • The build never needs to know the diff ID or PHID. In particular, no diff ID will exist when building commits.
dpkp added a comment.Aug 8 2016, 4:35 PM

Apologies, I should have added more detail to the original task! I am actually trying to use just the staging push to trigger remote tests, and not require each project to setup and maintain herald rules + harbormaster build plans that call out to HTTP endpoints w/ parameters.

The workflow I am hoping for is:

  • arc diff
    • creates a new diff id
    • attaches diff to existing revision or creates new revision
    • pushes diff to staging area as phabricator/diff/##
  • staging push triggers a test run (this step external to phabricator)
  • update unit test results via external test run
    • get diff id from staging tag (phabricator/diff/##)
    • using diff id, lookup harbormaster phid using conduit api calls
      • phid.lookup
      • harbormaster.querybuildables
      • harbormaster.queryautotargets [hoping to use 'arcanist.unit' here]
    • harbormaster.sendmessage

Some additional history: I have been using phabricator since before harbormaster days. We installed it many years ago at Rdio. We originally used a custom arcanist workflow to put unit test results into POSTPONED state, trigger a remote build, and have the remote build update the unit test results. When harbormaster was released and this functionality was removed, we switched our code to use herald + harbormaster build plans. I found that the overhead of managing herald + harbormaster was much higher than our previous setup and was causing friction getting folks to adopt phabricator for new projects. Rdio has since shutdown and many of us have transitioned to Pandora. I'm currently trying to get a minimal-friction phabricator workflow.

epriestley added a comment.Aug 8 2016, 4:49 PM

I found that the overhead of managing herald + harbormaster was much higher than our previous setup and was causing friction getting folks to adopt phabricator for new projects

Can we fix this problem (or set of problems) instead? The workflow you describe is not something we're interested in spending resources supporting since it is much more limited than the Herald + Harbormaster flow in the general case.

dpkp added a comment.Aug 8 2016, 5:54 PM

Can we fix this problem (or set of problems) instead? The workflow you describe is not something we're interested in spending resources supporting since it is much more limited than the Herald + Harbormaster flow in the general case.

I think you could, but I don't have any great ideas for you on that front.

But so I'm also not asking you to support this workflow explicitly. Just for what I think is a fairly minor addition to the phid.lookup conduit api that I think is consistent with its current design. Barring a reason to not support phid lookups for diffs, it seems to me like a nice win-win. Also, I submitted a revision that I think implements the change; so hopefully not much work on your end for this at all!

dpkp added a comment.Aug 8 2016, 6:36 PM

Thinking a bit more about the herald / harbormaster approach, I think what might help that workflow, for me at least, is conduit API access to create herald rules and harbormaster build plans.

Would it be possible to have herald rules that fire when a new repository is created that can setup the required herald rules + harbormaster build plans?

Relatedly, conduit API access to add / configure a new diffusion repository?

Ultimately I'm hoping to get as close as possible to a script that will configure a new repository w/ all steps so that arc diff works and tests are triggered etc.

avivey added a subscriber: avivey.Aug 8 2016, 6:50 PM

Why does each repository require its own herald rules/hm builds, if those are not customized by the team?

dpkp added a comment.Aug 8 2016, 7:00 PM

The way I've configured it in the past is to have the build trigger an HTTP call to a parameterized jenkins job (passing the various vars required to update w/ harbormaster.sendmessage). In the past we've needed each team to have / manage a different jenkins job, and so we created a new build plan for each w/ a different HTTP endpoint to call.

Looking back at the harbormaster parameters, it looks like it would be possible to write a single abstract job that is parameterized by repository.staging.uri and ref as well as the target PHID. This would only work (I think) if each repository had a consistent test interface. But maybe that isn't that bad to manage internally. Is that how you've done it, avivey?

avivey added a comment.Aug 8 2016, 7:03 PM

In my setup, we have cross-repository tests, so there's maybe 4 jobs that need would run would each run when one of many repos change; So it's similar, except that I filter on "Repository Project Contains".

epriestley closed this task as Resolved.Aug 10 2016, 8:34 PM
  • We'll provide a third generation differential.diff.search endpoint eventually which will be able to do this lookup in a modern, consistent way, but have no particular timeline on this.
  • I'm open to accepting a patch to add 'phid' to differential.querydiffs in the meantime.
  • I don't want to bring "Diff X" upstream as an object name. This (English word + space) is inconsistent with other object names, ambiguous in some contexts like CLI revision messages, surprising in other contexts like fulltext search from the global menu, and faces more internationalization issues than existing monograms.
  • You can create repositories via third-generation API methods, see Diffusion User Guide: Repositories API for guidance.
  • See T9766 for Herald control via API, although neither that use case nor this use case are very strong from my perspective.
  • Harbormaster will get more API support eventually, but see below.

I think there's some prior discussion somewhere that I'm not having much luck digging up immediately, but if all of your projects run builds and tests in different ways that generally seems like a problem that impacts everyone working on all of the projects, not just Phabricator or automated integrations. That is, it's better for everyone if you can check out any project and immediately know how to build/test it than if this knowledge is stored outside the project in an inconsistent way. An obvious advantage is that automated tools can do this, but it's helpful for normal humans too.

One approach is to use arc as the translation point, and make arc unit work everywhere, then have your builds just run arc unit. This is possible-but-messy today, but should become easier in the future, after T5568 / T5055.

But any convention would work equally well -- put an automation/build.sh script in everything if you want. But the basic argument is that the knowledge of how to build a project should be contained in the project, versioned alongside the project, and exposed via a consistent API or script or config or tool, instead of contained on a wiki or in someone's head or in a private Jenkins script somewhere or in Harbormaster or in some script which configures Harbormaster via the API. A new hire should ideally be able to learn approximately one convention and then build every project you work with, and if you're debugging a month-old build you should just be able to check out the codebase without also having to go figure out how Jenkins was configured a month ago or look up the older version of a Wiki page or ask someone if they remember.

This may not be immediately practical in real environments, of course, and may not happen overnight, but it seems pretty desirable to me in the long run. In particular, I'd like to make arc easier to use as this centralization point in the future, so it's a reasonable choice if it makes sense in your environment. But regardless of whether you use arc or not, I think any development activity is likely to benefit from efforts that build toward standardization of how things are built and tested, even if that progress is gradual.

I think @avivey's solution (some finite number of mostly-standard builds, selected by tagging repositories with projects like "Use node Build Rules") is a good stepping stone or alternate-but-roughly-similar approach, and would let you build toward having fewer different types of build rules.

I'm going to mark this as resolved since I think everything has been covered in pretty good detail, but let me know if I'm missing anything. Happy to accept a 'phid' diff too if that's still valuable as a stopgap -- there's no technical or product reason not to expose it.

dpkp added a comment.Aug 10 2016, 9:59 PM

Thanks for the attention and detailed thoughts. I really appreciate it. I
agree with your comments re standardizing the test interface. This
resolution makes sense to me.

20after4 awarded a token.Jun 16 2017, 8:30 AM
• GoogleLegacy added a subscriber: • GoogleLegacy.Feb 19 2018, 4:32 AM
Phabricator · Built by Phacility