1130683 - gecko: Owner validation (email validations) should never cause task graph / push failures

Status: RESOLVED FIXED

(Taskcluster :: General, defect)

Comment 1

[James Lal [:lightsofapollo]]

This is an ongoing discussion ... Some of the bots seem to fail the email validator and other commits seem to have irregular emails (in github integration this is the wild west).

I think this comes down to the fact the _only_ the server side integration knows if it should be posting data for X user and that information does not always tie cleanly to a single "owner". For now I am going to set "owner" to a mailing list for tc (or my own). In addition adding the original "owner" flag to:

tags.createdForUser = <user>

<user> will mean github user likely in one case and whatever the value for email we get in the pushlog case.

Comment 2

[Jonas Finnemann Jensen (:jonasfj)]

For the record:
Github suggests that people use: <username>@users.noreply.github.com, if they want to hide their email.
See: https://help.github.com/articles/keeping-your-email-address-private/

We could do the same if github users don't have an email associated.
--
Granted I'm open to changing the requirement for "owner". So that the docs still says that it has to be an email, but that we don't validate this. So obscure owner strings will be accepted.
It's not like we have a strict use-case for it, or some method of validating that the person posted the task really has the given email.

But if we could rely on pseudo emails as above, it is in my opinion nicer to have a field that is guaranteed to be an email. Even if the email may be fake or invalid.

Comment 3

[James Lal [:lightsofapollo]]

Attachment: MozReview Request: bz://1130683/lightsofapollo

/r/3593 - Bug 1130683 - Use stable owner name which will never fail

Pull down this commit:

hg pull review -r cbfd20edbf008ff6700885107f0aff4f2fd96727

Comment 4

[James Lal [:lightsofapollo]]

Comment on attachment 8561503 [details]
MozReview Request: bz://1130683/lightsofapollo

/r/3593 - Bug 1130683 - Use stable owner name which will never fail
/r/3595 - Bug 1131152 - Update tester image to use tc-vcs and download mozharness r=garndt
/r/3603 - Bug - 1131154 Use fake buildprops file to bypass use of buildbotconfigs in mozharness r=garndt
/r/3605 - Bug 1130696 - Update pull gaia script to use correct version based on gaia.json in gecko r=garndt
/r/3607 - Bug 1131156 - Add tc-vcs cache to tests r=garndt
/r/3609 - Bug 1124340 - Add buildbot step lines to tester image r=garndt
/r/3611 - Bug 1131160 - Remove caching of gaia due to issues updating clones on tester r=garndt
/r/3613 - Bug - 1131164 Add mozharness generated logs as artifacts r=garndt
/r/3615 - Bug - 1131165 Increase xpcshell timeout to closer to 2 hours r=garndt
/r/3617 - Bug 1131166 - Update mochitest symbol to use tc prefix r=garndt
/r/3619 - Bug 1131169 - Attempt to enable some emulator-kk tests r=garndt

Pull down these commits:

hg pull review -r bd41dd3464778cefa40c547fba8c4fd871da41d6

Comment 5

[Jonas Finnemann Jensen (:jonasfj)]

@jlal, if we're just going to always hardcode and not make the fallback dependent on whether or not we have a valid email for the user. Then we should just drop email validation and let people stuff whatever in owner, but ask (in the docs) that the owner field is an email or something similar.

Comment 6

[Greg Arndt [:garndt]]

Comment on attachment 8561503 [details]
MozReview Request: bz://1130683/lightsofapollo

https://reviewboard.mozilla.org/r/3591/#review2821

::: testing/taskcluster/mach_commands.py
(Diff revision 2)
> +    gaia = json.load(open(os.path.join(GECKO, 'b2g/config/gaia.json')))

Probably doesn't matter, but we could just do os.path.join(GECKO, 'b2g', 'config'...) so it uses os.sep as the directory separator

::: testing/taskcluster/mach_commands.py
(Diff revision 2)
> +       gaia['git']['remote'] == '' or \

Not sure it really matter (or if it would happen), but if gaia.json has a key for git, but not for one of the others that this checks for, this will result in a key error being thrown.

::: testing/taskcluster/mach_commands.py
(Diff revision 2)
> -        parameters = {
> +        parameters = dict(gaia_info().items() + {

Is there a situation where gaia.json has 'git' info, but you want it to use the integration branch in hg?

Comment 7

[Greg Arndt [:garndt]]

Comment on attachment 8561503 [details]
MozReview Request: bz://1130683/lightsofapollo

https://reviewboard.mozilla.org/r/3591/#review2831

Ship It!

Comment 8

[Greg Arndt [:garndt]]

Just a few questions, but have been monitoring try with these changes and as far as I can see so far it looks ok.

Comment 9

[James Lal [:lightsofapollo]]

> Probably doesn't matter, but we could just do os.path.join(GECKO, 'b2g', 'config'...) so it uses os.sep as the directory separator

Will fix prior to landing...


>Not sure it really matter (or if it would happen), but if gaia.json has a key for git, but not for one of >the others that this checks for, this will result in a key error being thrown.

I _think_ this is okay as it should fail the decision task in a user visible way.

Comment 10

[James Lal [:lightsofapollo]]

https://hg.mozilla.org/integration/b2g-inbound/rev/0e2316747aaf

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0e2316747aaf

Comment 13

[James Lal [:lightsofapollo]]

Attachment: MozReview Request: Bug 1131166 - Update mochitest symbol to use tc prefix r=garndt

Comment 14

[James Lal [:lightsofapollo]]

Attachment: MozReview Request: Bug 1131169 - Attempt to enable some emulator-kk tests r=garndt

Comment 15

[James Lal [:lightsofapollo]]

Attachment: MozReview Request: Bug 1130683 - Use stable owner name which will never fail

Comment 16

[James Lal [:lightsofapollo]]

Attachment: MozReview Request: Bug 1131152 - Update tester image to use tc-vcs and download mozharness r=garndt

Comment 17

[James Lal [:lightsofapollo]]

Attachment: MozReview Request: Bug - 1131154 Use fake buildprops file to bypass use of buildbotconfigs in mozharness r=garndt

Comment 18

[James Lal [:lightsofapollo]]

Attachment: MozReview Request: Bug 1130696 - Update pull gaia script to use correct version based on gaia.json in gecko r=garndt

Comment 19

[James Lal [:lightsofapollo]]

Attachment: MozReview Request: Bug 1131156 - Add tc-vcs cache to tests r=garndt

Comment 20

[James Lal [:lightsofapollo]]

Attachment: MozReview Request: Bug 1124340 - Add buildbot step lines to tester image r=garndt

Comment 21

[James Lal [:lightsofapollo]]

Attachment: MozReview Request: Bug 1131160 - Remove caching of gaia due to issues updating clones on tester r=garndt

Comment 22

[James Lal [:lightsofapollo]]

Attachment: MozReview Request: Bug - 1131164 Add mozharness generated logs as artifacts r=garndt

Comment 23

[James Lal [:lightsofapollo]]

Attachment: MozReview Request: Bug - 1131165 Increase xpcshell timeout to closer to 2 hours r=garndt

Comment 24

[Pete Moore [:pmoore][:pete]]

Resetting Version and Target Milestone that accidentally got changed...