Page MenuHomePhabricator

Support builds with Travis CI
Closed, WontfixPublic

Description

Problem we're trying to solve:
We're integrating 3rd party CI service (Travis) with Phabricator.

Travis works in following way:

  • monitor the remote repo
  • on every commit to a remote branch, check the .travis.yml file if the branch matches given regexp. If so, put a worker task in a queue
  • once a task gets assigned to a worker, run the shell script defined in the configuration file
  • if the script exits with code 0, assume tests passed, if exits with non-zero, assume tests failed.

The script is an arbitrary shell script, that we have 100% control over.

We'd like the Travis worker to pass the test results to Phabricator.

Currently, we have configured it in following way:

  • whenever a user creates or updates a diff, we push a new branch to a remote repo
  • Travis picks up the branch and spawns the tests
  • simultaneously, using Herald, we hook up a Harbormaster build plan
  • the build plan waits for a message from Travis worker before it finishes

Main problem: Travis worker need to get the PHID for the Build Target to report the test results using harbormaster.sendmessage call. The PHID is only available while executing the Build Step. There is no way to query for it using Conduit Harbormaster API.

We mitigate that by creating a paste, with PHID of the Build Target as a content. Then we use paste.query API to find the correct PHID. Obviously, this is a nasty hack, and we'd like to get rid of it. With introduction of differential.setdiffproperty, it's possible to store the PHID inside a travis-phid property using a HTTP request to Conduit. Unfortunately, Travis worker has no way to retrieve it.

One can fix it in several ways:

  • Add an API to Harbormaster that allows to obtain the Build Target PHID,
  • Use a 3rd party service that stores the PHID for us,
  • Keep the PHID inside a diff properties, and allow the user to retrieve them.

FWIW, the API seems not intuitive for me: why does the user is able to set arbitrary properties via Conduit, if there is no way to get them back? Is exposing the API actually a hack, to achieve other goals?

OLD CONTENT

Reproduction steps:

  • Add diff property using differential.setdiffproperty API
$ echo '{
  "diff_id": 123,
  "name": "foo",
  "data": "\"bar\""
}' | arc call-conduit --conduit-uri https://secure.phabricator.com/ --conduit-token <conduit-token> differential.setdiffproperty
  • Use differential.querydiffs to find the diff:
$ echo '{
  "ids": [
    123
  ]
}' | arc call-conduit --conduit-uri https://secure.phabricator.com/ --conduit-token <conduit-token> differential.querydiffs

Expected result:

  • The "foo": "bar" property should appear somewhere in the returned object

Actual result:

  • There is no "foo": "bar" property anywhere in the object.

This can be fixed by either adding needProperties(true) here, or by creating a differential.getdiffproperties conduit API call.

This was mentioned in the last comment in T9334.

Affected version: 8a2afa14d24f3cc9feaefbf62e65e0a8f3cc5333

Event Timeline

This isn't a bug. Diff properties are not intended to be an arbitrary key-value datastore for API consumers.

Please read Contributing Feature Requests and Describing Root Problems, then rewrite this as a feature request which describes the problem you're trying to solve. In particular, your description should be general enough that it's possible for us to suggest alternatives and workarounds.

FWIW, the API seems not intuitive for me: why does the user is able to set arbitrary properties via Conduit, if there is no way to get them back? Is exposing the API actually a hack, to achieve other goals?

This allows newer versions of arc to work properly with older versions of Phabricator. For example, D15426 added a new property, arc.staging. Because the API allows the client to write properties it doesn't know about yet, users who upgraded arc could continue working with older versions of Phabricator.


On Travis, see some discussion in T9456.

If Harbormaster does not trigger the build, this system may race (Travis may complete a build before Harbormaster generates a target).

Travis support is a very low priority for us. It may be possible to use autotargets, but you're pretty much on your own. The suggested approach where diff properties are used as a key/value store is too hacky to come upstream, although you're free to write your own Conduit method exposing properties.

epriestley renamed this task from There is no way to read diffproperty using Conduit to Support builds with Travis CI.Dec 13 2016, 8:47 PM
epriestley triaged this task as Wishlist priority.
epriestley added a project: Harbormaster.

If Harbormaster does not trigger the build, this system may race (Travis may complete a build before Harbormaster generates a target).

I consider this negligible -- Travis build takes at least a minute, because of machine startup times. I've never seen Harbormaster generating the targert taking over a minute.

Travis support is a very low priority for us. It may be possible to use autotargets, but you're pretty much on your own. The suggested approach where diff properties are used as a key/value store is too hacky to come upstream, although you're free to write your own Conduit method exposing properties.

We'll probably create our own Conduit method then. Thanks!

You could also ask Travis for a build.run sort of API. If you're a paying customer of theirs, it might be easier for them to prioritize this request than it is for us to prioritize Travis support. With such an API, there's a fairly straightforward path to Phabricator upstream support.

(Earlier, several CircleCI customers were able to convince them to implement a minor required change to their similar API in just seven months.)

You could also ask Travis for a build.run sort of API. If you're a paying customer of theirs, it might be easier for them to prioritize this request than it is for us to prioritize Travis support. With such an API, there's a fairly straightforward path to Phabricator upstream support.

We don't have 7 months to wait for that feature ;). Please find the diffusion.getdiffproperties API call attached.

FWIW: Travis is working on a build.run API: https://docs.travis-ci.com/user/triggering-builds

Unfortunately, it won't be possible to trigger such builds with Phabricator right now, as Phabricator does not allow to control HTTP Request headers and body.

We're OK with bringing custom build steps for popular platforms upstream, like HarbormasterCircleCIBuildStepImplementation for CircleCI.

(See also T9608 for the more general version of that task.)

epriestley claimed this task.

TravisCI sold to Idera and is no longer "cool".

I'm generally moving Phabricator away from "free glue for paid systems", see T13229, so this is unlikely to ever come upstream.

We might provide a licensed TravisCI integration some day, but there isn't a significant amount of customer interest today.

Third parties can build this integration; or ask TravisCI to build this integration; or pay us to build this integration ahead of T13229; after T5055 this will all become easier.