Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tcdownload: returns 1 if 'No complete TaskCluster runs found for ref' #19187

Conversation

psaavedra
Copy link
Contributor

Typically in UNIX like systems, every command returns an exit semantic status (sometimes referred to as a return status or exit code). A successful command returns a 0, while an unsuccessful one returns a non-zero value that usually can be interpreted as an error code. I propose a small change in the script to return 1 in 'No complete TaskCluster runs found for ref' situations.

Copy link
Member

@foolip foolip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know where tcdownload.py is used, so @jgraham should probably review.

@@ -71,7 +71,7 @@ def run(*args, **kwargs):

if not taskgroups:
logger.error("No complete TaskCluster runs found for ref %s" % kwargs["ref"])
return
return 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't sys.exit(run(None, vars(kwargs))) or similar needed below for this to work?

Copy link
Contributor Author

@psaavedra psaavedra Sep 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The result of the run() function for tc-download is captured by wpt in

sys.exit(int(rv))
)

Actually, the tc-download is a subcommand of wpt. Example:

./wpt tc-download --ref a7f3642cf9af7321211ac05cd06e0512fb4d6372  ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW @clopez has an interesting extension for this which we are using to get results from task-cluster and upload it the wpt.fyi repository: psaavedra@c093e31

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, then I understand the context. Writing a longish explanation in the commit message is OK and helpful in review.

@jugglinmike
Copy link
Contributor

It's a sub-commant for the WPT CLI:

$ ./wpt tc-download --help
usage: wpt tc-download [-h] [--ref REF] [--artifact-name ARTIFACT_NAME]
                       [--repo-name REPO_NAME] [--token-file TOKEN_FILE]
                       [--out-dir OUT_DIR]

optional arguments:
  -h, --help            show this help message and exit
  --ref REF             Branch (in the GitHub repository) or commit to fetch
                        logs for
  --artifact-name ARTIFACT_NAME
                        Log type to fetch
  --repo-name REPO_NAME
                        GitHub repo name in the format owner/repo. This must
                        be the repo from which the TaskCluster run was
                        scheduled (for PRs this is the repo into which the PR
                        would merge)
  --token-file TOKEN_FILE
                        File containing GitHub token
  --out-dir OUT_DIR     Path to save the logfiles

@foolip
Copy link
Member

foolip commented Sep 20, 2019

What uses ./wpt tc-download? I guessed wpt.fyi but that's not it, so maybe Gecko CI?

@jgraham
Copy link
Contributor

jgraham commented Sep 21, 2019

I don't know that any part of our automation is currently using this. That doesn't mean it's not useful of course.

@foolip foolip merged commit ad8bc5b into web-platform-tests:master Sep 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants