New issue
Advanced search Search tips

Issue 624545 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 600469



Sign in to add a comment

Value of "master" in Rietveld API "try_job_results" is sometimes prefixed with "master.", sometimes not.

Project Member Reported by qyears...@chromium.org, Jun 29 2016

Issue description

If I run `git cl try` with a master that's not prefixed by "master.", like this:

  git cl try -m tryserver.chromium.linux -b linux_chromium_rel_ng

Then in the results from the Rietveld JSON API (e.g. https://codereview.chromium.org/api/2112603002/1) the "master" field in "try_job_results" is "tryserver.chromium.linux".

However, if I start it giving like this:

  git cl try -m master.tryserver.chromium.linux -b mac_chromium_rel_ng

Then "master" in the new entry for the new try job is "master.tryserver.chromium.linux".


According to https://cs.chromium.org/chromium/tools/depot_tools/git_cl.py?l=234, the string with the "master." prefix is the "full" master name, and Rietveld bucket name; the version without that prefix the master name commonly used by developers.

It may be nice to normalize the master name in the Rietveld JSON.
 
Summary: Value of "master" in Rietveld API "try_job_results" is sometimes prefixed with "master.", sometimes not. (was: Value of "master" in Rietveld API "try_job_results" can sometimes prefixed with "master.", sometimes not.)
Status: Untriaged (was: Unconfirmed)
Blockedon: 600469
Cc: no...@chromium.org tandrii@chromium.org
Status: Available (was: Untriaged)
Owner: tandrii@chromium.org
Status: WontFix (was: Available)
Hm, please don't use Rietveld API for tryjob results, use buildbucket instead.
That said, I have to fix git cl to disallow master names without "master." prefix ( issue 637031 ). CQ has already been fixed yesterday. 

Comment 5 by no...@chromium.org, Aug 11 2016

"tryserver.chromium.linux" is a full master name, but not full bucket name. A full bucket name is "master.tryserver.chromium.linux"

I think --master should accept master name without "master." prefix. "tryserver.chromium.infra" is a full master name, not to be confused with full bucket name. A full bucket name is "master.tryserver.chromium.infra".

I think we should leave --master for backward compatibility reasons (not prepend "master." prefix) and add -B/--bucket flag that accepts only full bucket name

tandrii, thoughts?
Agree! Copying your mesage to  issue 637031 .
Sounds good -- for clarification: is `git cl` still the preferred tool for triggering try jobs?

Comment 8 by no...@chromium.org, Aug 12 2016

i guess it depends. if this is a script that is running on a dev machine, then by using git cl the script will reuse cached credentials

if this runs on a bot, i am not sure what credentials git cl upload will use. in that case it might be cleaner to use a service account (i am not sure git cl supports that), which means you will have to use buildbucket api directly
Makes sense -- for the particular case I'm interested in now, "w3c-test-autoroller", it has Rietveld credentials on the bot itself, stored in a particular place in the filesystem, and that's used when running git cl try (https://cs.chromium.org/chromium/src/third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py?l=81).

It also uses these credentials when uploading and committing CLs, which is part of what it's used for.

It sounds like if it were just triggering try jobs, using a buildbucket service account would be cleaner, but not sure if that's the case if it also needs to upload patches and trigger commits.
If you trigger jobs on CLs, then IMO git cl should be the default way. The reason for this is that there are several properties that ideally should be set on all tryjob CLs so that common recipe modules (like gclient and bot_update) can do the right thing for your tryjobs. It's easier to keep this properties set up in 1 place than scattered across many custom service launchers.

Now, if git cl doesn't support service account credentials, that's a bug and should be fixed. 

Rietveld credentials, in the form of refresh tokens[1] are already deployable using puppets to specific machines. So your current way is 100% the right way to do it, so  long as the credentials file isn't manually copied to machine by you.

[1] I think refresh token can be generated for service accounts as well, so no bug in git cl, but i didn't check.
Thanks Andrii, makes sense :-)

Sign in to add a comment