1016117 - Have treeherder emit pulse messages when resultsets are created so TC tasks can be attached

Status: RESOLVED FIXED

(Tree Management :: Treeherder, enhancement, P4)

Description

[James Lal [:lightsofapollo]]

The current taskcluster + try integration creates it's own resultset and jobs. This is fine if we only wanted to create tasks and resultsets from TC but that is not the case. For some time we will need to support the current buildbot flow and other tooling that will eventually report into treeherder.

A easy solution to this is to have treeherder simply emit a pulse message when a resultset is created.

Something like:

exchange: treeherder/v1/resultset
routingKey: created.$repoName

With the current body of the resultset.

This will allow us to easily attach jobs to resultsets from different sources without racing to create the resultset (with potentially different data) and provides a much easier source of information for new commits (from try, github, hg, etc.., etc...).

Comment 1

[James Lal [:lightsofapollo]]

We can probably work around not having this but this will make standing up TC alongside everything else almost trivial and would help other projects as well I am sure.

Comment 2

[Jonas Finnemann Jensen (:jonasfj)]

I've hacked a proposal for this here:
https://github.com/mozilla/treeherder-service/pull/220

It needs the following:
 A) Proper naming of exchange, routing key entries, documentation
    I have a very limited idea about what data I'm actually posting, so a well-defined
    formalization of what various names cover and what they reference would be nice.
    What is 'revision'? and 'revision_hash'? which is the result-set-id, is there such a thing?
    And if there is no result-set-id, how is result-set identified? Should we call the exchange
    something else? Assuming there is no such thing as a result-set-id.
 B) 'repository_id' should be resolved to something, what? how?
 C) Additional comments to JSON schema, perhaps additional formalization
 D) Reorganizing the code here, I just added pulse_publisher.py somewhere, along with exchanges.py
    (maybe pulse_publisher.py belongs in a utilities/helper folder, and exchanges maybe somewhere else)
 E) Configure namespace for publisher in tasks.py, should be treeherder-local, treeherder-dev,
    treeherder-staging or treeherder for local, dev, stage and production setting.

It would be great if someone from treeherder team could take a look at (A) through (E), I'll happily answer an questions. But I have limited idea about how treeherder works and what the data model looks like. So most of the documentation I wrote is pure guess work :)

Remark:
 i)  This uses the public pulse user, from my understanding PulseGuardian is far from working,
     and no multi-user support.
 ii) This doesn't use confirm-channels, it might be possible to do with python, but my experiments
     with pyamqp only resulted in SSL not working, but still no blocking on publish.

Future work:
 1) Publish JSON schemas,
 2) Publish JSON reference as automatically generated by pulse_publisher.py
 3) Static web-site with auto-generated docs from reference and JSON schemas (I have code for this)

Comment 3

[Jonas Finnemann Jensen (:jonasfj)]

I've fixed (B) thanks to jeads.

I'm now using this for developing locally.
Note. (E) should be addressed before this merged, but it's just a configuration issue.

Comment 4

[Jonas Finnemann Jensen (:jonasfj)]

Attachment: treeherder-pulse.patch

See PR: https://github.com/mozilla/treeherder-service/pull/220
Note that there are two commits in this PR, the first one installs additional dependencies, see attached patch for the interesting commit, or:
https://github.com/jonasfj/treeherder-service/commit/0b560bbf3ebbd6a2603120a1e1478ab6daa29ed3

The other commit it the vendored stuff.

Basically, the only thing that IMO blocks this is that we need to configure the `namespace` value differently on production, staging, development and local deployments.
See:
https://github.com/jonasfj/treeherder-service/commit/0b560bbf3ebbd6a2603120a1e1478ab6daa29ed3#diff-977722f4425644b9c2ed7229bc4477ebR112

If you could help me get the `namespace` property defined in configuration, that would be great. It'll also needs a review, if so let me know if you have any comments, please let me know... I would like to get this landed soon, it's been blocking my deployment of taskcluster-try integration for a while now.

Note, this will need some extra reconfiguration, when pulse guardian goes operational with multi-user support. We should ask mcote about how to make publishers when that happens.
I suspect it just adding username/password to the configuration file and getting rid of the `namespace` property. This will probably just be a configuration issue, ie. no new code.

Comment 5

[Mauro Doglio [:mdoglio]]

Sorry if it's taking so long to give you a feedback, I'm planning to do it within the end of the week.

Comment 6

[Mauro Doglio [:mdoglio]]

Comment on attachment 8497009 [details] [diff] [review]
treeherder-pulse.patch

Review of attachment 8497009 [details] [diff] [review]:
-----------------------------------------------------------------

It looks great, can you please make it pep8 compliant?

::: treeherder/model/tasks.py
@@ +106,5 @@
> +schemas = load_schemas(schema_folder)
> +
> +publisher = TreeherderPublisher(
> +    'public', 'public', schemas,
> +    # TODO: Use treeherder-local, treeherder-dev, treeherder-staging for

There's a SITE_URL variable available in the settings file that you can use to derive an identifier for the environment in use.
This is tipically "http://local.treeherder.mozilla.org" for local development, http://treeherder-dev.mozilla.org for dev, etc. See https://docs.djangoproject.com/en/1.6/topics/settings/#using-settings-in-python-code for detail on how to access the django settings file.

Alternatively, you can create a new property in treeherder/settings/settings.py,
give it a default value and overwrite it in your local settings file. You can then access it the same way as before via django.conf.settings

Comment 7

[Jonas Finnemann Jensen (:jonasfj)]

Comments should be addressed now.

kombu update have been moved to compiled.txt as requested by jeads on irc, and submitted in a separate PR:
https://github.com/mozilla/treeherder-service/pull/238

Note, you'll need to more PR 238, before you merge PR 220.

@mdoglio (or jeads),
Is there anything else I can do to make this land?
Just getting it on staging will allow us to deploy our try hookup code (already implemented and tested against his patch).

Comment 8

[Treeherder GitHub Bugbot]

Commits pushed to master at https://github.com/mozilla/treeherder-service

https://github.com/mozilla/treeherder-service/commit/94bbbfb183d7a5ad7aa99d6f84a083135eb5d850
Bug 1016117 - Added/updated vendor libs to support publishing to pulse

https://github.com/mozilla/treeherder-service/commit/6dc451372ba70537c412fcd1bdd6fa0a9e8e8d9f
Bug 1016117 - update celery and kombu version in requirements

https://github.com/mozilla/treeherder-service/commit/b9d40fb40a773d4a1724d6eff94a2cec46c69117
Bug 1016117 - update celery project layout and worker init script

https://github.com/mozilla/treeherder-service/commit/ec6f20d588216e438ba048f54a594667f91d9f6e
Merge pull request #239 from mozilla/bug-1016117-vendor-updates

Bug 1016117 - Added/updated vendor libs to support publishing to pulse

Comment 9

[Jonas Finnemann Jensen (:jonasfj)]

Attachment: Migration to use pulseguardian users

This should be a easy thing to review...

You can look at the last commit as it is all the difference from what you reviewed before.
Basically, it just not using the namespace and instead of using public pulse user it uses a username/password loaded from config file.

Note, I've tested this locally and messages does arrive. So it works.

Comment 10

[Mauro Doglio [:mdoglio]]

Comment on attachment 8504983 [details] [review]
Migration to use pulseguardian users

I added a few comments to the PR

Comment 11

[Jonas Finnemann Jensen (:jonasfj)]

Attachment: Migration to use pulseguardian users

Addressed logging, import issues, employed try/finally and responded to the rest of you questions.

I tested this locally and it works for me. I see the messages on pulse...

Comment 12

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder-service

https://github.com/mozilla/treeherder-service/commit/2f39d4353c535f774cc7558c46da068152543bb1
Merge pull request #250 from jonasfj/migrate-to-pulse-guardian

Migrate to pulse guardian (bug 1016117)

Comment 13

[Mauro Doglio [:mdoglio]]

Thanks for the great job Jonas! Sorry if it took so long to review it.
I'll put it on stage on Monday

Comment 14

[Jonas Finnemann Jensen (:jonasfj)]

That's great, once on stage we can finally deploy out try integration bits.

Please make sure the staging exchange is listed here:
https://wiki.mozilla.org/Auto-tools/Projects/Pulse/Exchanges

Otherwise it can be really hard to guess the exchange name which depends in the username you registered.

Comment 15

[Mauro Doglio [:mdoglio]]

It's on stage now, :jonasfj can you confirm it's working?

Comment 16

[Jonas Finnemann Jensen (:jonasfj)]

Sorry, I missed your comment, yes, I can confirm that it's working...
First real-world use-case for pulse inspector:
https://tools.taskcluster.net/pulse-inspector/

@mdoglio,
Can we add you as "maintainer" for the exchanges on:
https://wiki.mozilla.org/Auto-tools/Projects/Pulse/Exchanges