1944522 - Send JSON content type through Harbormaster Build step

Status: RESOLVED FIXED

(Conduit :: Phabricator, defect)

Description

[Bastien Abadie [:bastien]]

I'm working on triggering a Taskcluster hook from Phabricator upon new diffs.

The triggerHookWithToken API endpoint requires to be called with:

• HTTP POST query
• HTTP header Content-Type: application/json
• Empty body is accepted

The Harbormaster HTTP build step is able to send POST queries, but does not set any header.

I saw that the HTTPSFuture class has a addHeader method that would allow to send that header.

Would you consider a patch on the Mozilla Phabricator fork that:

• adds an optional field there to specify a Content-type header
• uses that field value to configure HTTPSFuture

Comment 1

[:glob ✱]

👍 That sounds like a small change with a large impact; we'd be ok taking a patch that provides this.

Comment 2

[Bastien Abadie [:bastien]]

Thanks for the quick reply :glob, I made a PR with the changes needed to send a Content-Type header. This is indeed a small patch !

Comment 3

[Bastien Abadie [:bastien]]

I also experimented to add an optional HTTP body to the same build step, allowing to merge the variables (same as in the URI).

With a similar patch as the content-type, I'm able to trigger taskcluster hook and providing the Phabricator PHIDs I need for the bot.

:dkl mentionned a while back that Phabricator upstream refused to implement this kind of change (although in a more complete manner).
Would you consider such a patch though ? It would allow us to trigger the hook for the code review bot without needing any change on the Taskcluster side

Comment 4

[David Lawrence [:dkl]]

Merged
https://github.com/mozilla-conduit/phabricator/commit/b2487f553628bb524b610d6f5c8f90ac448b7a95

Will be in the next Phab deployment very soon.

Comment 5

[David Lawrence [:dkl]]

This is on phabricator-dev if you want us to get everything set up for testing there. I filed bug 1944974 to have production deployed this coming Monday.

Comment 6

[BMO Github Automation]

Attachment: [mozilla-conduit/phabricator] Bug 1944522 - Add HTTP Body option in Harbormaster HTTP Build step (#49)

Comment 7

[Bastien Abadie [:bastien]]

Thanks a lot for the merge & deployment. I actually need this other patch to trigger the Taskcluster hook.

I can file another bug if you prefer to keep this one closed.