1741976 - parse_scalars.py throws if type check of expires field fails

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P1)

Description

[Emma Zühlcke [:emz]]

When I pass an integer instead of a string for the "expires" key in Scalars.yaml the parser fails instead of providing a meaningful validation error.

Here is the trace:

[task 2021-11-18T18:18:53.373Z] 18:18:53    ERROR -  Traceback (most recent call last):
[task 2021-11-18T18:18:53.374Z] 18:18:53     INFO -    File "/usr/lib/python3.9/runpy.py", line 197, in _run_module_as_main
[task 2021-11-18T18:18:53.374Z] 18:18:53     INFO -      return _run_code(code, main_globals, None,
[task 2021-11-18T18:18:53.374Z] 18:18:53     INFO -    File "/usr/lib/python3.9/runpy.py", line 87, in _run_code
[task 2021-11-18T18:18:53.374Z] 18:18:53     INFO -      exec(code, run_globals)
[task 2021-11-18T18:18:53.375Z] 18:18:53     INFO -    File "/builds/worker/checkouts/gecko/python/mozbuild/mozbuild/action/file_generate.py", line 156, in <module>
[task 2021-11-18T18:18:53.375Z] 18:18:53     INFO -      sys.exit(log_build_task(main, sys.argv[1:]))
[task 2021-11-18T18:18:53.375Z] 18:18:53     INFO -    File "/builds/worker/checkouts/gecko/python/mozbuild/mozbuild/action/util.py", line 18, in log_build_task
[task 2021-11-18T18:18:53.375Z] 18:18:53     INFO -      return f(*args, **kwargs)
[task 2021-11-18T18:18:53.376Z] 18:18:53     INFO -    File "/builds/worker/checkouts/gecko/python/mozbuild/mozbuild/action/file_generate.py", line 100, in main
[task 2021-11-18T18:18:53.376Z] 18:18:53     INFO -      ret = module.__dict__[method](
[task 2021-11-18T18:18:53.377Z] 18:18:53     INFO -    File "/builds/worker/checkouts/gecko/toolkit/components/telemetry/build_scripts/gen_scalar_data.py", line 207, in main
[task 2021-11-18T18:18:53.377Z] 18:18:53     INFO -      scalars = parse_scalar_definitions(filenames)
[task 2021-11-18T18:18:53.377Z] 18:18:53     INFO -    File "/builds/worker/checkouts/gecko/toolkit/components/telemetry/build_scripts/gen_scalar_data.py", line 161, in parse_scalar_definitions
[task 2021-11-18T18:18:53.377Z] 18:18:53     INFO -      batch = parse_scalars.load_scalars(filename)
[task 2021-11-18T18:18:53.378Z] 18:18:53     INFO -    File "/builds/worker/checkouts/gecko/toolkit/components/telemetry/build_scripts/mozparsers/parse_scalars.py", line 500, in load_scalars
[task 2021-11-18T18:18:53.378Z] 18:18:53     INFO -      ScalarType(category_name, probe_name, scalar_info, strict_type_checks)
[task 2021-11-18T18:18:53.379Z] 18:18:53     INFO -    File "/builds/worker/checkouts/gecko/toolkit/components/telemetry/build_scripts/mozparsers/parse_scalars.py", line 43, in __init__
[task 2021-11-18T18:18:53.379Z] 18:18:53     INFO -      self.validate_types(definition)
[task 2021-11-18T18:18:53.380Z] 18:18:53     INFO -    File "/builds/worker/checkouts/gecko/toolkit/components/telemetry/build_scripts/mozparsers/parse_scalars.py", line 168, in validate_types
[task 2021-11-18T18:18:53.380Z] 18:18:53     INFO -      wrong_type_names = [
[task 2021-11-18T18:18:53.381Z] 18:18:53     INFO -    File "/builds/worker/checkouts/gecko/toolkit/components/telemetry/build_scripts/mozparsers/parse_scalars.py", line 169, in <listcomp>
[task 2021-11-18T18:18:53.381Z] 18:18:53     INFO -      "{} must be {}".format(f, ALL_FIELDS[f].__name__)
[task 2021-11-18T18:18:53.382Z] 18:18:53     INFO -  AttributeError: 'tuple' object has no attribute '__name__'
[task 2021-11-18T18:18:53.382Z] 18:18:53    ERROR -  gmake[3]: *** [backend.mk:684: toolkit/components/telemetry/.deps/TelemetryScalarData.h.stub] Error 1

Comment 1

[Chris H-C :chutten]

ALL_FIELDS['expires'] is six.string_types which, according to its source is str in PY3 and basestring in PY2. str is not a tuple, as far as I can tell. It's a class which means it should have a __name__.

And yet. If, in my mozilla build environment, I crack open python3 and import six and then ask it what six.string_types is, it reports (<class 'str'>,).

This is a double mystery because 1) How does it get to be a tuple 2) If it's a tuple, how does the isinstance typecheck ever work if it's checking str vs tuple of str?

Comment 2

[Jan-Erik Rediger [:janerik]]

(In reply to Chris H-C :chutten from comment #1)

ALL_FIELDS['expires'] is six.string_types which, according to its source is str in PY3 and basestring in PY2. str is not a tuple, as far as I can tell. It's a class which means it should have a __name__.

According to that source string_types is str, <- see that trailing comma. That means it really is (str,), a tuple, as the REPL also tells you/

And yet. If, in my mozilla build environment, I crack open python3 and import six and then ask it what six.string_types is, it reports (<class 'str'>,).

This is a double mystery because 1) How does it get to be a tuple 2) If it's a tuple, how does the isinstance typecheck ever work if it's checking str vs tuple of str?

1. solved.
2. as per the docs:

If classinfo is a tuple of type objects (or recursively, other such tuples) or a Union Type of multiple types, return True if object is an instance of any of the types.

So it checks for str really, the first and only element of the tuple.

So really the only issue is that we unconditionally use .__name__ on that.
We could do str(ALL_FIELDS[f]) as a quick fix:

>>> import six
>>> six.string_types[0].__name__
'str'
>>> str(six.string_types)
"(<class 'str'>,)"

not the nicest, but maybe good enough for now (alternatively refactor that into a helper method that checks if it's a tuple and if so concatenates the __name__s of each tuple element)

Comment 3

[Chris H-C :chutten]

Thank you, that helps a bunch! I'm all for a quick fix.

(( And I think that str, with a leading meaningful comma is a bad syntax and should feel bad. But I was fooled by it, so I'm probably not the most objective judge. ))

Comment 4

[Chris H-C :chutten]

Attachment: Bug 1741976 - Improve Scalar and UserInteraction parser errors about types r?janerik!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0e4052072b4c
Improve Scalar and UserInteraction parser errors about types r=janerik

Comment 6

[Marian-Vasile Laza]

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