Run glean_parser for linting metrics.yaml in mozilla-central
Categories
(Data Platform and Tools :: Glean: SDK, enhancement, P1)
Tracking
(Not tracked)
People
(Reporter: Dexter, Unassigned)
References
Details
(Whiteboard: [telemetry:glean-rs:backlog])
We currently have metrics.yaml
in m-c which is used by the Glean SDK when GeckoView is used in Glean-aware products.
We should make sure this file is linted using the glean_parser and that we break build if it isn't, otherwise we will end up breaking Android Components and figuring that out when is too late.
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Comment 1•5 years ago
|
||
Hi :glob!
I'm not sure if you're the right person for this answer, would you kindly redirect if you're not?
We need to call glean_parser (Python 3.7, to be vendored or fetched when setting up the environment) at build time, in order to lint this file and fail build (or a test?) if errors are detected.
Is there any way to invoke python3 scripts from the build system?
Comment 2•5 years ago
|
||
I believe we have the ability to run tests with Python 3, that's probably the thing to do here. :ahal may have more specific advice.
Reporter | ||
Updated•5 years ago
|
Comment 3•5 years ago
|
||
The Python 3.7 requirement will definitely complicate matters. I believe both the build and python-tests run with 3.5 in CI. If this is something you want to integrate as part of the build itself you'll need to reduce the glean_parser
requirement down to 3.5 full stop (I'm not a build peer, this is just my understanding). If it's something external to the build (i.e run as part of make check
or as a separate task), then you can likely continue using 3.7 but may run into issues where workers don't have that installed.
I agree a python-test seems the most ideal (we do run some of them in make check
atm, though I don't believe we run any under Python 3 yet). Other possibilities are a linter, or a standalone task in taskcluster/ci/source-test
.
Reporter | ||
Comment 4•5 years ago
|
||
(In reply to Andrew Halberstadt [:ahal] from comment #3)
The Python 3.7 requirement will definitely complicate matters. I believe both the build and python-tests run with 3.5 in CI. If this is something you want to integrate as part of the build itself you'll need to reduce the
glean_parser
requirement down to 3.5 full stop (I'm not a build peer, this is just my understanding).
@mshal, flagging you as Chris is away. Do you know what's the Python3 version our build system is using? On Windows, MozillaBuild
seems to be shipping with Python 3.7.4, but I could not find any specific version reference in our code, other than "Something >= 3.5".
Comment 5•5 years ago
|
||
(In reply to Alessio Placitelli [:Dexter] from comment #4)
(In reply to Andrew Halberstadt [:ahal] from comment #3)
The Python 3.7 requirement will definitely complicate matters. I believe both the build and python-tests run with 3.5 in CI. If this is something you want to integrate as part of the build itself you'll need to reduce the
glean_parser
requirement down to 3.5 full stop (I'm not a build peer, this is just my understanding).@mshal, flagging you as Chris is away. Do you know what's the Python3 version our build system is using? On Windows,
MozillaBuild
seems to be shipping with Python 3.7.4, but I could not find any specific version reference in our code, other than "Something >= 3.5".
Right, 3.5 is the minimum python3 version currently required. On our build machines, it looks like we have these exact versions installed:
Linux: 3.5.3
Mac: 3.5.3
Windows: 3.6.5
Note that the scripts invoked during the build currently use python2.7, though work is underway to start using py3 instead. See eg: bug 1599658
Reporter | ||
Comment 6•5 years ago
|
||
(In reply to Michael Shal [:mshal] from comment #5)
Right, 3.5 is the minimum python3 version currently required. On our build machines, it looks like we have these exact versions installed:
Thanks. Looks like our main blocker is depending on Python 3.7, then :)
@Mike, is there any way to use Python 3.5 instead of Python 3.7 in glean_parser? How much do we rely on 3.7 features?
Comment 7•5 years ago
|
||
I haven't done an exhaustive search, but the main Python 3.7-only feature we use is the dataclasses
library. It looks like there is a backport for that to Python 3.6 here, but not all the way back to 3.5.
Comment 8•5 years ago
|
||
Also see this relevant issue about supporting dataclasses on Python 3.5. We would probably be better off rewriting glean_parser to not use dataclasses
, which could definitely be done without breaking external API, there would just be a lot more boilerplate.
Reporter | ||
Comment 9•5 years ago
|
||
Hi Chris,
how likely would it be to bump the minimum Python build version of Firefox to 3.6/3.7 in order to support this? Would it be a reasonable request? I'm fairly sure this has many dependencies that I'm not aware of, but I wanted to try asking anyway :)
Reporter | ||
Comment 10•5 years ago
|
||
FWIW, looks like Python 3.5 will be EOL by September 2020
Comment 11•5 years ago
|
||
I don't know if we'd go as far as requiring a different version to build for this, but there's no prohibition against using more recent versions in tests that run in CI. Would it be sufficient to run this test per-checkin on one platform (Linux for instance)? It may be feasible to set up a docker image with the right python version for a specific test.
Reporter | ||
Comment 12•5 years ago
|
||
(In reply to Chris Manchester (:chmanchester) from comment #11)
I don't know if we'd go as far as requiring a different version to build for this, but there's no prohibition against using more recent versions in tests that run in CI.
Thank you for following up. We will need to add this as part of the build system in the future, as we want feature parity with our current Firefox Telemetry: it breaks the build if something weird is detected in our registry files.
We'll go on and port to Python 3.5.3.
Reporter | ||
Comment 13•5 years ago
|
||
Untaking: this is currently blocked by dependent bugs.
Reporter | ||
Comment 14•5 years ago
|
||
Works for this is happening in the bug being duplicated against.
Description
•