Matthew N. [:MattN] Reporter
	Description • 11 years ago

Any queued or future talos runs on Try seem to break because of this change. This makes it harder to bisect performance regressions using Try server.

18:29:55    FATAL - Uncaught exception: Traceback (most recent call last):
18:29:55    FATAL -   File "C:\slave\test\scripts\mozharness\base\script.py", line 1048, in run
18:29:55    FATAL -     self.run_action(action)
18:29:55    FATAL -   File "C:\slave\test\scripts\mozharness\base\script.py", line 990, in run_action
18:29:55    FATAL -     self._possibly_run_method(method_name, error_if_missing=True)
18:29:55    FATAL -   File "C:\slave\test\scripts\mozharness\base\script.py", line 931, in _possibly_run_method
18:29:55    FATAL -     return getattr(self, method_name)()
18:29:55    FATAL -   File "C:\slave\test\scripts\mozharness\mozilla\testing\talos.py", line 449, in download_and_extract
18:29:55    FATAL -     self._populate_webroot()
18:29:55    FATAL -   File "C:\slave\test\scripts\mozharness\mozilla\testing\talos.py", line 390, in _populate_webroot
18:29:55    FATAL -     talos_repo = self.query_talos_repo()
18:29:55    FATAL -   File "C:\slave\test\scripts\mozharness\mozilla\testing\talos.py", line 256, in query_talos_repo
18:29:55    FATAL -     return self.talos_json_config['global']['talos_repo']
18:29:55    FATAL - KeyError: 'talos_repo'
18:29:55    FATAL - Exiting -1

	Aki Sasaki (not active)
	Comment 1 • 11 years ago

I think we can work around this by adding a default of http://hg.mozilla.org/build/talos .

	Jason Yeo[:jyeo]
	Comment 3 • 11 years ago

Comment on attachment 778435 [details] [diff] [review]
talos.diff

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

::: mozharness/mozilla/testing/talos.py
@@ +254,5 @@
>          """
> +        default_repo = "http://hg.mozilla.org/build/talos"
> +        try:
> +            if self.query_talos_json_config():
> +                return self.talos_json_config['global']['talos_repo']

Not sure about the usage of "try...except" here. I would prefer

return self.talos_json_config['global'].get('talos_repo', default_repo)

@@ +256,5 @@
> +        try:
> +            if self.query_talos_json_config():
> +                return self.talos_json_config['global']['talos_repo']
> +            else:
> +                return self.config.get('talos_repo')

Same here. self.config.get('talos_repo', default_repo)

	Armen [:armenzg]
	Comment 4 • 11 years ago

I used the try/except clause to avoid having to write this (which I think we should) and saw as ugly:
self.talos_json_config.get('global', {}).get('talos_repo', default_repo)

I am getting worried about the case where the talos.json's would not contain "global" in it.

	Aki Sasaki (not active)
	Comment 5 • 11 years ago

Comment on attachment 778453 [details] [diff] [review]
talos.diff

If you're worried about global going away, your comment string is fine, too.

	Armen [:armenzg]
	Comment 6 • 11 years ago

Comment on attachment 778453 [details] [diff] [review]
talos.diff

http://hg.mozilla.org/build/mozharness/rev/01eac888fc98

	Armen [:armenzg]
	Comment 7 • 11 years ago

This is live now.

	Matthew N. [:MattN] Reporter
	Comment 8 • 11 years ago

Thanks for the quick fix!