1118758 - Autophone - support testing try builds

Status: RESOLVED FIXED

(Testing Graveyard :: Autophone, defect)

Description

[Bob Clary [:bc] (inactive)]

In order to meet developer and sheriff needs, Autophone needs to be able to test try builds.

Comment 1

[Bob Clary [:bc] (inactive)]

Attachment: example.txt

Example BuildConsumer output for a try build.#.finished.

For this bug, I'm interested in the comment/commit_title
"try: -b o -p android-api-9,android-api-11 -u autophone-mdm,autophone-s1s2 -t none"

This doesn't appear to be available in the pulsebuildmonitor or the NormalizedBuildConsumer and at the moment requires me to use a BuildConsumer. I can implement what I need for Autophone and perhaps gain additional information that I can use to submit to Treeherder. I could also use the additional information to get the symbols and tests from pulse rather than having to munge the buildurl to guess the symbols or tests zips.

mcote, this is what I plan to do for this bug.

mcote, dkl: I wonder about providing the comment/commit_title and perhaps other informaion to other non-buildbot frameworks where dispatching off of the try commit message would be useful. It is not clear to me what the best approach would be to expose this in general. My first impression is the NormalizedBuildConsumer should expose these additional properties.

Comment 2

[Mark Côté [:mcote]]

dkl hasn't been involved with Pulse lately.

Providing the commit is probably a good idea.  dminor, how did you handle this with Autoland?

Comment 3

[Dan Minor [:dminor]]

I was looping over the changes and looking at the comments field, like so:

for change in payload['build']['sourceStamp']['changes']:
    comments = change['comments']

where payload = data['payload'] in the pulse handle_message callback.

I was using the BuildConsumer because the NormalizedBuildConsumer was missing stuff that I needed.

Comment 4

[Bob Clary [:bc] (inactive)]

sounds like the same approach I was using. Ok. I'll just go forward and we can think about the general case later I guess.

Comment 5

[Bob Clary [:bc] (inactive)]

Attachment: bug-1118758-try-builds-1-v1.patch

autophonepulsebuildmonitor.py

Comment 6

[Bob Clary [:bc] (inactive)]

Attachment: bug-1118758-try-builds-2-v1.patch

autophone.py

This doesn't yet discriminate by the tests specified in the try commit message but will run any configured tests for any matching try build.

example run with mozilla-inbound, try and try: -b o -p android-api-9,android-api-11 -u autophone-mdm,autophone-s1s2 -t none

http://phonedash-dev.allizom.org/#/org.mozilla.fennec/throbberstop/local-twitter/norejected/2015-01-16/2015-01-16/notcached/errorbars/standarderror

Comment 7

[Bob Clary [:bc] (inactive)]

I left an extraneous self.pulsemonitor = None in the Autophone constructor in patch 2. Removed locally.

Comment 8

[Bob Clary [:bc] (inactive)]

Attachment: bug-1118758-try-builds-1-v2.patch

While performing a long running test, the message.ack() raised an SSLError. I've added a try except around message.ack to prevent any exception raised during the ack from causing the worker to die.

Comment 9

[Bob Clary [:bc] (inactive)]

Attachment: bug-1118758-try-builds-2-v2.patch

This patch removes the autophonepulsemonitor attribute from Autophone and just uses the existing pulsemonitor attribute.

Comment 10

[Mark Côté [:mcote]]

Comment on attachment 8550683 [details] [diff] [review]
bug-1118758-try-builds-1-v2.patch

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

::: autophonepulsebuildmonitor.py
@@ +15,5 @@
> +    messages rather than the normalized messages in order to obtain
> +    the check-in comment for a build. The comment is used to determine
> +    if a try build has requested Autophone testing.
> +
> +    :param build_callback: Required callback function which takes a

You should probably note that this is always called in a new thread.

@@ +39,5 @@
> +    :param buildtypes: Required list of build types to
> +        process. Possible values are 'opt', 'debug'
> +    :param logger: Required Logger instance.
> +    :param user: Pulse user name.
> +    :param password: Pulse password.

This should probably take a PulseConfiguration object so you can set other parameters like 'host' and such.

@@ +86,5 @@
> +        assert platforms, "platforms is required."
> +        assert buildtypes, "buildtypes is required."
> +        assert logger is not None, "logger is required."
> +
> +        self.build_callback= build_callback

Missing space.

@@ +102,5 @@
> +        `start` method to call `listen` from a thread since the
> +        call to BuildConsumer.listen blocks.
> +        """
> +        pulse_args = {
> +            'applabel': 'autophone-build-monitor',

I would say try to make this unique (and logged) in case someone tries to run two monitors with the same user, e.g. for testing.

@@ +104,5 @@
> +        """
> +        pulse_args = {
> +            'applabel': 'autophone-build-monitor',
> +            'topic': 'build.#.finished',
> +            'durable': False,

This should probably default to True (and be overrideable; see my comment about PulseConfiguration above).  If autophone goes down temporarily, you probably don't want to lose any try runs in that time, I would imagine.

@@ +113,5 @@
> +        build_consumer = BuildConsumer(**pulse_args)
> +
> +        while True:
> +            try:
> +                build_consumer.listen()

You *might* have to recreate build_consumer on subsequent iterations; I'm not sure if .listen() behaves properly if called multiple times on the same consumer object.

@@ +130,5 @@
> +        :param data: Buildbot pulse message data.
> +        :param message: Buildbot pulse message metadata.
> +        """
> +        try:
> +            message.ack()

I wonder if this might cause weirdness, since it's not called from the thread that listen() is running in.  Maybe move this to on_build_event() to be careful.

@@ +137,5 @@
> +            # the ssl library is used:
> +            # SSLError: [Errno 1] _ssl.c:1312: error:1409F07F:SSL routines:SSL3_WRITE_PENDING:bad write retry
> +            # Whatever error may occur it should not cause us to miss
> +            # processing the message or to cause the program to terminate.
> +            # Therefore, we will just log the error and continue.

I'm a little worried about this.  This might result in unacked messages building up and/or possibly multiple triggers from the same try run.

@@ +178,5 @@
> +            if property_name in fields:
> +                build_data[property_name] = type(property[1])(property[1])
> +
> +        for required_field in required_fields:
> +            if required_field not in build_data or not build_data[required_field]:

Should we log this?  Or are these common?

@@ +194,5 @@
> +            return
> +        if build_data['branch'] == 'try' and 'autophone' not in build_data['comments']:
> +            return
> +
> +        self.build_callback(build_data)

Any particular reason you aren't parsing the autophone comment here?

Comment 11

[Mark Côté [:mcote]]

Comment on attachment 8550684 [details] [diff] [review]
bug-1118758-try-builds-2-v2.patch

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

Ignore the item from my last review about parsing the try syntax.  I didn't realize that part was yet to come; thought you had put it in this patch.

Comment 12

[Bob Clary [:bc] (inactive)]

(In reply to Mark Côté [:mcote] from comment #10)
> Comment on attachment 8550683 [details] [diff] [review]
> bug-1118758-try-builds-1-v2.patch
> 
> Review of attachment 8550683 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: autophonepulsebuildmonitor.py
> @@ +15,5 @@
> > +    messages rather than the normalized messages in order to obtain
> > +    the check-in comment for a build. The comment is used to determine
> > +    if a try build has requested Autophone testing.
> > +
> > +    :param build_callback: Required callback function which takes a
> 
> You should probably note that this is always called in a new thread.
> 

Ok.

> @@ +39,5 @@
> > +    :param buildtypes: Required list of build types to
> > +        process. Possible values are 'opt', 'debug'
> > +    :param logger: Required Logger instance.
> > +    :param user: Pulse user name.
> > +    :param password: Pulse password.
> 
> This should probably take a PulseConfiguration object so you can set other
> parameters like 'host' and such.
> 

I'm not sure what this would give us. BuildConsumer doesn't take a PulseConfiguration object but instead uses the keyword args to call the PulseConfiguration initializer. I had originally used a PulseConfiguration object until I realized it wasn't supported and went back to the individual arguments. I suppose I could add keyword arguments for all of the PulseConfiguration arguments and pass them through but constructing the PulseConfiguration object itself wouldn't serve any purpose.

> @@ +86,5 @@
> > +        assert platforms, "platforms is required."
> > +        assert buildtypes, "buildtypes is required."
> > +        assert logger is not None, "logger is required."
> > +
> > +        self.build_callback= build_callback
> 
> Missing space.
> 
> @@ +102,5 @@
> > +        `start` method to call `listen` from a thread since the
> > +        call to BuildConsumer.listen blocks.
> > +        """
> > +        pulse_args = {
> > +            'applabel': 'autophone-build-monitor',
> 
> I would say try to make this unique (and logged) in case someone tries to
> run two monitors with the same user, e.g. for testing.
> 

Ok.

> @@ +104,5 @@
> > +        """
> > +        pulse_args = {
> > +            'applabel': 'autophone-build-monitor',
> > +            'topic': 'build.#.finished',
> > +            'durable': False,
> 
> This should probably default to True (and be overrideable; see my comment
> about PulseConfiguration above).  If autophone goes down temporarily, you
> probably don't want to lose any try runs in that time, I would imagine.
> 

Ok.

> @@ +113,5 @@
> > +        build_consumer = BuildConsumer(**pulse_args)
> > +
> > +        while True:
> > +            try:
> > +                build_consumer.listen()
> 
> You *might* have to recreate build_consumer on subsequent iterations; I'm
> not sure if .listen() behaves properly if called multiple times on the same
> consumer object.
> 

Ok.

> @@ +130,5 @@
> > +        :param data: Buildbot pulse message data.
> > +        :param message: Buildbot pulse message metadata.
> > +        """
> > +        try:
> > +            message.ack()
> 
> I wonder if this might cause weirdness, since it's not called from the
> thread that listen() is running in.  Maybe move this to on_build_event() to
> be careful.
> 

Ok.

> @@ +137,5 @@
> > +            # the ssl library is used:
> > +            # SSLError: [Errno 1] _ssl.c:1312: error:1409F07F:SSL routines:SSL3_WRITE_PENDING:bad write retry
> > +            # Whatever error may occur it should not cause us to miss
> > +            # processing the message or to cause the program to terminate.
> > +            # Therefore, we will just log the error and continue.
> 
> I'm a little worried about this.  This might result in unacked messages
> building up and/or possibly multiple triggers from the same try run.
> 

There is not a lot I can do here. I can try to call ack again, but there is no guarantee it will succeed in removing the message. There was a lot of discussion that I read that the way to recover was to re-submit the call and that openssl would handle it but it required calling the openssl functions with the exact same buffers not just the exact same content. That is something I can't control. The worst case I was considering was that I would end up processing the same message twice and possibly generating jobs for the same build twice. I'll add a retry and deal with the possible duplicate jobs in the job insertion in autophone.py. For what is worth, I only saw this once in several hours/dozen builds during testing.

> @@ +178,5 @@
> > +            if property_name in fields:
> > +                build_data[property_name] = type(property[1])(property[1])
> > +
> > +        for required_field in required_fields:
> > +            if required_field not in build_data or not build_data[required_field]:
> 
> Should we log this?  Or are these common?
> 

I'll add a log output during my testing. If they aren't common, I'll leave it in.

> @@ +194,5 @@
> > +            return
> > +        if build_data['branch'] == 'try' and 'autophone' not in build_data['comments']:
> > +            return
> > +
> > +        self.build_callback(build_data)
> 
> Any particular reason you aren't parsing the autophone comment here?

As you noticed, that is something I'm going to deal with in autophone.py and doesn't belong in this class I don't think.

Comment 13

[Mark Côté [:mcote]]

(In reply to Bob Clary [:bc:] from comment #12)
> > @@ +39,5 @@
> > > +    :param buildtypes: Required list of build types to
> > > +        process. Possible values are 'opt', 'debug'
> > > +    :param logger: Required Logger instance.
> > > +    :param user: Pulse user name.
> > > +    :param password: Pulse password.
> > 
> > This should probably take a PulseConfiguration object so you can set other
> > parameters like 'host' and such.
> > 
> 
> I'm not sure what this would give us. BuildConsumer doesn't take a
> PulseConfiguration object but instead uses the keyword args to call the
> PulseConfiguration initializer. I had originally used a PulseConfiguration
> object until I realized it wasn't supported and went back to the individual
> arguments. I suppose I could add keyword arguments for all of the
> PulseConfiguration arguments and pass them through but constructing the
> PulseConfiguration object itself wouldn't serve any purpose.

You can work around this by passing connect=False and then setting the PulseConfiguration object directly; see what I did to pulsetranslator, pulsebuildmonitor, and such.  Don't get me wrong--it's pretty horrible--but it's useful for testing and such, until we get a better Pulse Python package.  The advantage of this is that you don't need to duplicate the PulseConfiguration args in your code, in case they change.

> > @@ +137,5 @@
> > > +            # the ssl library is used:
> > > +            # SSLError: [Errno 1] _ssl.c:1312: error:1409F07F:SSL routines:SSL3_WRITE_PENDING:bad write retry
> > > +            # Whatever error may occur it should not cause us to miss
> > > +            # processing the message or to cause the program to terminate.
> > > +            # Therefore, we will just log the error and continue.
> > 
> > I'm a little worried about this.  This might result in unacked messages
> > building up and/or possibly multiple triggers from the same try run.
> > 
> 
> There is not a lot I can do here. I can try to call ack again, but there is
> no guarantee it will succeed in removing the message. There was a lot of
> discussion that I read that the way to recover was to re-submit the call and
> that openssl would handle it but it required calling the openssl functions
> with the exact same buffers not just the exact same content. That is
> something I can't control. The worst case I was considering was that I would
> end up processing the same message twice and possibly generating jobs for
> the same build twice. I'll add a retry and deal with the possible duplicate
> jobs in the job insertion in autophone.py. For what is worth, I only saw
> this once in several hours/dozen builds during testing.

Okay, avoiding duplicate triggers in general is probably the best way to handle this.

Comment 14

[Bob Clary [:bc] (inactive)]

(In reply to Bob Clary [:bc:] from comment #12)
> (In reply to Mark Côté [:mcote] from comment #10)

> > @@ +178,5 @@
> > > +            if property_name in fields:
> > > +                build_data[property_name] = type(property[1])(property[1])
> > > +
> > > +        for required_field in required_fields:
> > > +            if required_field not in build_data or not build_data[required_field]:
> > 
> > Should we log this?  Or are these common?
> > 
> 
> I'll add a log output during my testing. If they aren't common, I'll leave
> it in.
> 

Actually, this is going to happen for any non android build which won't have a packageUrl field. I'm going to leave it out completely.

(In reply to Mark Côté [:mcote] from comment #13)

> (In reply to Bob Clary [:bc:] from comment #12)
> > > @@ +39,5 @@
> 
> You can work around this by passing connect=False and then setting the
> PulseConfiguration object directly; see what I did to pulsetranslator,
> pulsebuildmonitor, and such.  Don't get me wrong--it's pretty horrible--but
> it's useful for testing and such, until we get a better Pulse Python
> package.  The advantage of this is that you don't need to duplicate the
> PulseConfiguration args in your code, in case they change.

Ok. I see how that works. Thanks.

> 
> > > @@ +137,5 @@
> 
> Okay, avoiding duplicate triggers in general is probably the best way to
> handle this.

I don't understand.

Comment 15

[Mark Côté [:mcote]]

(In reply to Bob Clary [:bc:] from comment #14)
> > You can work around this by passing connect=False and then setting the
> > PulseConfiguration object directly; see what I did to pulsetranslator,
> > pulsebuildmonitor, and such.  Don't get me wrong--it's pretty horrible--but
> > it's useful for testing and such, until we get a better Pulse Python
> > package.  The advantage of this is that you don't need to duplicate the
> > PulseConfiguration args in your code, in case they change.
> 
> Ok. I see how that works. Thanks.

Just want to note again that I know how ugly this hack is. :)

> > > > @@ +137,5 @@
> > 
> > Okay, avoiding duplicate triggers in general is probably the best way to
> > handle this.
> 
> I don't understand.

I was just agreeing with your plan to "deal with the possible duplicate jobs in the job insertion in autophone.py", which I interpreted as a generic fix for accidental job duplication from any source.

Comment 16

[Bob Clary [:bc] (inactive)]

Attachment: bug-1118758-try-builds-1-v3.patch

I don't use a randomly created applabel name since it wouldn't be possible to reconnect to a durable queue if a restart was needed.

Comment 17

[Bob Clary [:bc] (inactive)]

Attachment: bug-1118758-try-builds-2-v3.patch

Adds applabel and durable queue options as well as restores the use PulseConfiguration.

Comment 18

[Bob Clary [:bc] (inactive)]

Comment on attachment 8551298 [details] [diff] [review]
bug-1118758-try-builds-1-v3.patch

I messed up the durable setting for queues.

Comment 19

[Bob Clary [:bc] (inactive)]

Attachment: bug-1118758-try-builds-1-v4.patch

Comment 20

[Bob Clary [:bc] (inactive)]

Attachment: bug-1118758-try-builds-2-v4.patch

Comment 21

[Bob Clary [:bc] (inactive)]

https://github.com/mozilla/autophone/commit/768e17b7586b095c78ebcc930419006ef9373c3d
https://github.com/mozilla/autophone/commit/91b0bcaca9d2fcd04720b9db2739eef8fe92c4ae

Comment 22

[Bob Clary [:bc] (inactive)]

Attachment: bug-1118758-phonedash-v1.patch

This adds a control to allow the selection of try builds. You can see this in action at http://phonedash-dev.allizom.org. I also changed the checkbox controls to selects to make the UI appear more consistent and to simplify the code a bit.

Comment 23

[Mark Côté [:mcote]]

Comment on attachment 8551970 [details] [diff] [review]
bug-1118758-phonedash-v1.patch

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

lgtm

Comment 24

[Bob Clary [:bc] (inactive)]

Attachment: TypeError due to a property with value None

Saw this in production today after deploying the autophonepulsemonitor patch.

Comment 25

[Bob Clary [:bc] (inactive)]

(In reply to Mark Côté [:mcote] from comment #23)
> Comment on attachment 8551970 [details] [diff] [review]
> bug-1118758-phonedash-v1.patch

I missed one checkbox. I'll refresh the patch before commiting.

diff --git a/html/scripts/app.js b/html/scripts/app.js
index d28415b..3132c10 100644
--- a/html/scripts/app.js
+++ b/html/scripts/app.js
@@ -151,7 +151,7 @@ function makePlot(params, data) {
       points: {
           show: true,
           errorbars: 'y',
-          yerr: {show: $('#errorbars')[0].checked, upperCap: '-', lowerCap: '-'}
+          yerr: {show: params.errorbars, upperCap: '-', lowerCap: '-'}
       },
       lines: { show: true }
     },

Comment 26

[Bob Clary [:bc] (inactive)]

https://github.com/markrcote/phonedash/commit/3a7366ca47f436f2a1d0cd5e5be2d89273733424

Comment 27

[Bob Clary [:bc] (inactive)]

https://github.com/markrcote/phonedash/commit/9445981ea64fc106503f343d2ccc639fbf22d9ae

Comment 28

[Bob Clary [:bc] (inactive)]

Attachment: bug-1118758-try-builds-3-v1.patch

Fix for TypeError. I've been running this in production and it resolves the issue.

Comment 29

[Bob Clary [:bc] (inactive)]

Attachment: bug-1118758-try-builds-4-v1.patch

Add selectable tests for jobs. This is a bit bigger than I wanted but a lot of it is in the USAGE.md.

I changed the new_job and trigger_jobs to use the build data object instead of just the build url and took you up on your offer to let me jsonize stuff to the db. ;-)

I also detect duplicate jobs and reject them.

I removed PhoneTest.test_this_repo and used a new worker property runnable_tests to collect the tests which are actually to be run depending on the repo, devices and tests which are selected. It makes things a bit simpler.

I also used *Test.name for the test name selection.

You can see a limited test run with only autophone-smoketest at https://treeherder.allizom.org/#/jobs?repo=try&revision=7f32489e96d8 for the nexus-one-3 and samsung-gs3-3 devices. Note the production autophone is also testing this build and is using all tests since it doesn't have this patch yet.

seth and glandium are already using try in production and this will help me keep up with the load if they can select only the tests they are interested in. When dealing with their stuff, I realized I am missing email notification. I'll do that in a follow up patch and get gbrown to review since you are on your way to sunnier climes.

Comment 30

[Mark Côté [:mcote]]

Comment on attachment 8553194 [details] [diff] [review]
bug-1118758-try-builds-4-v1.patch

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

::: autophone.py
@@ +524,5 @@
> +        self.logger.debug('PULSE BUILD FOUND %s' % msg)
> +        tests = []
> +        if msg['branch'] == 'try':
> +            # Autophone try builds will have a comment of the form:
> +            # try: -b o -p android-api-9,android-api-11 -u autophone-smoke,autophone-s1s2 -t none

Does this mean that autophone try jobs have to *only* be for autophone?  Like you can't kick off an android try run with some regular buildbot jobs plus autophone?

Comment 31

[Bob Clary [:bc] (inactive)]

No, that was not my intention. The comment is mostly to describe how autophone specific try test selection would work.

reTests = re.compile('try:.* -u (.*) -t.*')
match = reTests.match(msg['comments'])
if match:
    tests = [t for t in match.group(1).split(',')
             if t.startswith('autophone-')]
    if 'autophone-tests' in tests:
        tests = []

should ignore any non-autophone tests and just pick up the autophone tests for example with this comment created by my local try chooser (with patch)

try: -b o -p android-api-9,android-api-11 -u plain-reftest-1,autophone-smoketest -t none

Buildbot will deal with the non-autophone test and ignore autophone while autophone will deal with its tests and ignore buildbots. I haven't tested it though. Think it is worth a test?

Comment 32

[Bob Clary [:bc] (inactive)]

Ok, kicked off a test with 

try: -b o -p android-api-9,android-api-11 -u crashtest-1,crashtest-2,autophone-smoketest,autophone-s1s2,autophone-webapp,autophone-mochitest-dom-browser-element,autophone-mochitest-dom-media,autophone-mochitest-skia,autophone-mochitest-toolkit-widgets -t none

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d2e38c7916ac

There is a backlog on autophone production but my local devices will be more responsive once the build completes.

Comment 33

[Bob Clary [:bc] (inactive)]

I screwed up and didn't turn on unittest downloads for the previous try build. I terminated the run early for my local setup and resubmitted another try with https://treeherder.mozilla.org/#/jobs?repo=try&revision=d626abf2d50b

I've been bitten by failing to enable downloading the tests zip before and have an idea for a fix, but will defer it to later.

Comment 34

[Bob Clary [:bc] (inactive)]

And the staging url to see Autophone results:
https://treeherder.allizom.org/#/jobs?repo=try&revision=d626abf2d50b

Comment 35

[Bob Clary [:bc] (inactive)]

https://github.com/mozilla/autophone/commit/24b0722dca3e63cb422076837e927bbd8fa18dcb
https://github.com/mozilla/autophone/commit/fadc14c774ef406aead459a0d14f3ba7e69e1591

Comment 36

[Bob Clary [:bc] (inactive)]

The patches so far have been deployed to production Autophone.

Comment 37

[Bob Clary [:bc] (inactive)]

Hey everyone, I wanted to get your feedback on email notifications for Autophone try jobs.

Normal try build notification appears to be limited to the initial email about your job with links to treeherder and the build directory.

Currently, we report Autophone to treeherder.allizom.org but after Bug 1118879 is fixed we will begin reporting to treeherder.mozilla.org. You will be able to follow along by watching Treeherder as the tests execute.

It may be the case that this is insufficient for Autophone since there may be a considerable delay between when the build becomes available and when the tests are completed on the various devices.

Would you find it useful to be notified when:

1. Autophone is notified of the try build and which devices will test the build?

2. A device begins processing the build and either aborts or marks the tests as pending?

3. A device begins running a particular test on the build?

4. A device completes a particular test on the build?

5. A device completes all tests on the build?

If you don't care to be notified, then I'll call this bug FIXED. But if you would like email notifications, please let me know what level of notification you would or would not like.

thanks.

Comment 38

[Seth Fowler [:seth] [:s2h]]

I'm answering this with very limited experience with Autophone so far, but I think I'd be OK with no notifications as long as I can follow progress on Treeherder. I can imagine it potentially being useful to get automated emails about regressions, just as we get from Talos, though.

Comment 39

[Mike Hommey [:glandium]]

Same as Seth.

Comment 40

[Brad Lassey [:blassey] (use needinfo?)]

I generally don't want email notifications from any of my try pushes

Comment 41

[Mark Finkle (:mfinkle) (use needinfo?)]

I'll go with the herd for now. Once we see how this works in more real world uses, we might change our mind.

No need to block on that to start though.

Great work on this!

Comment 42

[Bob Clary [:bc] (inactive)]

Attachment: bug-1118758-phonedash-2-v1.patch

This adds the option to select only try builds. When comparing only try builds, this make life much better since you don't have to de-select the regular builds each time you change a parameter.

Comment 43

[James Willcox (:snorp) ([e-mail]) (he/him)]

I'm cool with no notifications. Thanks Bob!

Comment 44

[Bob Clary [:bc] (inactive)]

Thanks everyone.

I've turned on reporting to https://treeherder.mozilla.org but with Autophone hidden. You can use the hidden toggle box to set exclusion_state=all so you can see Autophone jobs.

The UI/phonedash for try builds totally needs work and I still have to write the post about the latest Autophone news, but I'm calling this fixed.

Comment 45

[Bob Clary [:bc] (inactive)]

Attachment: bug-1118758-phonedash-2-v2.patch

Corrects mistakes I made in main/setControls for the try parameter which caused a linked url such as

http://phonedash.mozilla.org/#/org.mozilla.fennec/throbberstart/local-blank/norejected/2015-01-28/2015-01-28/notcached/noerrorbars/standarderror/try 

to actually load

http://phonedash.mozilla.org/#/org.mozilla.fennec/throbberstart/local-blank/norejected/2015-01-28/2015-01-28/notcached/noerrorbars/standarderror/notry

Comment 46

[Bob Clary [:bc] (inactive)]

Attachment: bug-1118758-try-builds-5-v1.patch

This fixes the phonedash url in the treeherder job details panel to accommodate the new url syntax for try builds.

Comment 47

[Bob Clary [:bc] (inactive)]

I've gone ahead and pushed these to fix the outstanding issues. Sorry for the lack of process.

You can see a test run at:
https://treeherder.allizom.org/#/jobs?repo=try&revision=e9110dd9d8b5

https://github.com/markrcote/phonedash/commit/c71a61f68033fe8b001f57065b703287ff856691
https://github.com/mozilla/autophone/commit/0222138ae6b158eacf4c9ac01e17574032f40399