New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 905834 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

[Telemetry] cv2 version check does not work on Chromebooks

Project Member Reported by cmt...@chromium.org, Nov 15

Issue description

In src/third_party/catapult/telemetry/telemetry/internal/util/external_modules.py there is code that tries to verify that cv2 has the right version.  However it makes assumptions about the format of the version that are not correct on Chromebooks.

The code tries to parse the module.__version___ (for cv2, numpy, and psutil) to make sure the version falls between the correct lower & upper bounds. This check was consistently failing on chromebooks, so I stuck in some print statements to verify what the module.__version__ looked like:

module: numpy, version: '1.9.2'
module: psutil, version: '2.2.1'
module: cv2, version: '$Rev: 4557 $'

(If you care, the code I inserted was:

  print("module: %s, version: %s" % (module.__name__, repr(module.__version__))) 


I inserted it in ImportRequiredModule, just after:

 module = importlib.import_module(module)

My current workaround is to insert:

 if module.__name__ == 'cv2':
    return module

just after that (otherwise telemetry dies with an ImportError). This really needs to be fixed.
 
Does Chromebooks have vpython? If yes, then the runnable script will trigger downloading the correct version, preventing this problem from happening
vpython does not appear to be on my chromebook.
cmtice@: you can get vpython from depot_tools

Comment 4 Deleted

That seems to me to be beside the point.  If vpython is not installed by default on Chromebooks, then telemety on Chromebooks should not require/expect it, should it?

We can update Telemetry to error out if the machine doesn't have vpython already if that what you mean in term of the user interface.

In term of code complexity, we will not support Telemetry without vpython since that just makes managing native python package a lot harder

Comment 7 Deleted

So what you're saying is that Telemetry will no longer be supported on Chromebooks?
What I mean is in order to use Telemetry, users would need to have vpython (through keeping a copy of depot_tools/ on their machines - http://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/depot_tools_tutorial.html#_setting_up). 

If one can install Telemetry on Chromebooks, they should also be able to have depot_tools on Chromebooks?

Comment 10 Deleted

I'm not really sure about the feasibility of what you're suggesting.  I'm Cc'ing vapier@ and dgarrett@, who might be able to give more informed feedback.

Comment 12 Deleted

Comment 13 Deleted

I assume the CPU architecture of the chromebooks in question is intel? I don't think we've built e.g. numpy for armv6l/arm64, in case that's an issue here.

vpython allows the script writers (in this case, telemetry) to dictate the python dependencies that they require. If this is a one-off application (i.e. a dev doing something locally, but not part of a main development workflow that they plan to maintain for weeks/months, or part of a bot workflow), it's possible to bypass vpython and take wheels from the system python (or a manually curated virtualenv) by setting the envvar:

  VPYTHON_BYPASS="manually managed python not supported by chrome operations"

(yes, the whole string). This is interpreted here: https://chromium.googlesource.com/chromium/tools/depot_tools/+/master/vpython#7, and also by the vpython go binary (if that's in $PATH ahead of depot_tools).
i don't know about this particular bug/setup, but we def run hwtests on arm/aarch64 CPUs.

Chromebooks have never had depot_tools available in it let alone vpython.  the last time this came up ( issue 843279 ), the test framework took care of deploying its unusual dependencies itself.
cmtice@: if you want to move this bug forward, can you book a meeting with me, Caleb & Robbie?
I do want to move this forward, and I can schedule the meeting. But although my team (the toolchain team) *uses* telemetry on Chromebooks all the time, our team does NOT have much to do with what does or does not get installed on Chromebooks, nor with maintaining the Chrome OS testing infrastructure.  I will see if I can find some appropriate people in those areas to invite as well.  I might also invite someone from the chromeos performance team, which also relies heavily on running Telmetry on Chromebooks.
Cc: cywang@chromium.org
Cc: akes...@chromium.org zamorzaev@chromium.org
+ the current deputies who are more familiar with what's happening in the lab now.

vpython will never be on ChromeBooks in the root OS, but might be installed on the stateful partition at test time.

There has been on going debate about python vs vpython vs virtualenv, but I don't know the current state of that discussion, so passing off to the deputy.

python - (ChromeOS) system management team ensures that the globally installed pip packages satisfy the requirements of all python scripts that will ever run on the machine.

virtualenv - (ChromeOS) system management team ensures one virtualenv for every unique set of
requirements for python scripts that will ever run on the machine, and explicitly choose the
one to use based on the script that is about to run (somehow).

vpython - scripts declare their own dependencies, and vpython ensures that such
a virtualenv exists, then runs the script in that virtualenv. The downside is that all wheels that any script needs must be built by this script (https://chromium.googlesource.com/infra/infra/+/master/infra/tools/dockerbuild/wheel.py) and uploaded to CIPD so that virtualenv assembly on bots only uses pre-built wheels, instead of requiring e.g. a compiler toolchain and python-dev to exist at the time the virtualenv is created.


In chrome land we're using vpython because it wasn't feasible for our system management team to manage either a global python installation, or a series of pre-made virtualenvs. If vpython doesn't work for chromeos, it would be good to understand why (last I knew it was potentially working, but running cbuildbot under it wasn't, but I don't think I ever knew the reason for that other than "it's complicated").
My two cents:
ChromeOS should be considered to be a different target than Linux. There's real work involved in porting and validation of any Linux tool to ChromeOS. For example, the glibc versions of ChromeOS and gLinux are not the same, so you at least need recompilation. 

In general, telemetry can rely on some tooling (since many of our tests already use that). But if telemetry is using something new, we have to make sure that there's tooling for it on Chrome OS, and its tested and validated to be working on ChromeOS and integrated into the testing. 


Cc: lakshm...@chromium.org tcwang@chromium.org
what is the status here?
Can we find an owner for this bug?

I am waiting for #17
Cc: vovoy@chromium.org
+vovoy
Vovo, could you help to take a look at this issue?
We are still using the opencv 2.3.0 in ChromeOS environment.
https://csearch.corp.google.com/chromeos_public/src/third_party/chromiumos-overlay/media-libs/opencv/?q=opencv+package:%5Echromeos_public$&dr
That's why we will get the cv2 version of '$Rev: 4557 $' (it is the version string before opencv 2.4.0).

Mike,
do you know who the right owner for the uprev of opencv is?

the owners are whoever use the package.  so prob the team that maintains the camera software stack in general.
Cc: jcliang@chromium.org hungte@chromium.org
Hi Ricky and Hungte,

  Not sure your team has any plan to uprev the opencv package? The one we have in cros repo seems pretty old. It's definitely helpful to uprev the opencv to   benefit all test frameworks.
Cc: -hungte@chromium.org stimim@chromium.org
+stimim for opencv in factory
stimim has not been active in the last 30 days (according to crosbug).
Is he/she the right person to assign this to?
offline chatted with cywang@, and didn't have a conclusion.

For factory team, we don't have immediate plan to do the migration of opencv package (porting opencv 3.x into chromeos and resolve build issues, etc), it's more like a P2 or P3 for us.
But we are okay to migrate our factory code to 3.x if someone wants to migrate the opencv package.

Do we have a target date that we want 3.x in chromeos system?
Depends on the date, we might be able to assign someone in our team to work on this, for example, 2019Q2.
stimim@, this is a bug that needs immediate attention. 
If you are the users of the library, it should be your team the one to update the package. Do you have any idea about how difficult is to update to latest version?
if updating to > 3.x? can  you update to some version of 2.4. Maybe this is quite simple to do?

nednguyen@, Ned: what about accepting '$Rev: 4557 $' for chromeos? This seems to be working fine...


Sorry, what is '$Rev: 4557 $'?
IIUC the problem is not just 'accepting' the version on ChromeOS, it's making it so that the code in catapult can be written against a specific (modern?) version of cv2. Accepting an old version of cv2 from the environment means that at least one of the following is true:
  * catapult can only be written against the oldest version supported
  * catapult would need to include graceful degradation logic to let it disable features based on the oldness of the libraries
  * or, catapult will break in confusing/exciting ways when the system provides a version that's too old.

I can potentially build vpython wheels for manylinux-arm64/manylinux-armv6l, but it sounds like the python being run here has an even more specialized OS target (chromeos?). How are wheels normally built for ChromeOS systems?

My other question would be; is there an alternate architecture where catapult can host these heavy dependencies in a more controlled environment? As it stands, if you wanted to support, say, mips, you have a lot of dependencies to carry with you.

Alternately, maybe there's a better way to ship a 'heavy' client? Maybe Go?

Probably all out of scope for this bug... but maybe worth thinking about.
If ChromeOS doesn't use image util in Telemetry, then I think it's fine to modify Telemetry so that it only check for cv2 existence when the image util is used.

Otherwise, I hesitate to add supporting different cv2 versions in Telemetry just because one client use a non supportive version. I truly think it's better for ChromeOS infra in the long term if you can figure out how to make vpython wheel work for ChromeOS testing.
If ChromeOS is using image utils and also using them on a platform where manylinux-x64 and manylinux-x86 don't work (e.g. if 'manylinux' is wrong for their python, or if x86/x64 are wrong for their supported CPU architectures), then we'll still need to figure out how this platform should be supported with vpython (assuming ChromeOS wants to use vpython in this case, which it sounds like is a ?????). If the python C extension abi is tied to the actual ChromeOS system libraries (including, for example, "libc"), then we may need to figure out a very different way to support this.

It's not at all clear to me who is using which libraries, of which versions, on which platforms though, or who should be supporting the resulting combinations, which is why I'm hesitant to prescribe any solutions.

If ChromeOS is indeed not actually using cv2, then what you propose in #34 sounds like a fine workaround.
> How are wheels normally built for ChromeOS systems?

they aren't ;)
So how is/was the cv2 package made available?

"building a wheel" is ~= "pip install". If you can 'pip install', you can make a wheel.
python packages are installed like normal packages.  we don't use pip either.  think `sudo apt-get install xxx`.
Ok, fine, and the apt package is ~= a wheel for the purposes of this discussion... wheels aren't magic, they're a zipfile that looks almost exactly like https://packages.ubuntu.com/trusty/amd64/python-opencv/filelist.

The hard part is compiling `cv2.so`, not whether it's in a pkg, zip, egg, whl, tarball or whatever.

So my question is (still), how/where are these built? Where are the build script(s)? Do they target 'manylinux', or do they target a specific linux platform variant? What architectures is it needed on? Is python here built with ucs2 or ucs4? etc.
i don't know why any of that matters.  the python packages built as part of the CrOS are guaranteed to work in that OS image.  why does the specific compilation mode matter ?
If catapult brings its own dependencies (vpython), it matters (because we need to build this). If ChromeOS is going to ensure it has up-to-date dependencies in order to run the latest version of catapult, it doesn't matter.
Then back to the original question, do we really need the cv2 w/ version number is greater than 2.4.0 in catapult? The opencv 2.3.0 in CrOS repo returns the version number w/ '$Rev: 4557 $' which is not correct format for comparison.
we could patch opencv in CrOS to report the right form ("2.3.0"), but if telemetry would still die because it wants 2.4.0+, then that wouldn't really help.

CrOS's python is built for USC4.  iiuc, "manylinux" and such is for wheel's to care about, not for packages built & installed as part of the OS image.
I think you're right; a manylinux-built extension could be loaded into any linux python interpreter as long as the shared libs in https://www.python.org/dev/peps/pep-0513/ on the system contains old-enough symbols to match (which they should).

UCS4 is good. This implies that the cv2 manylinux1 vpython wheels should at least work without special modification on X86/x64 if that was a desired route (for intel architectures).
What about the suggestion from Ned in #34:
"If ChromeOS doesn't use image util in Telemetry, then I think it's fine to modify Telemetry so that it only check for cv2 existence when the image util is used."

?
Labels: -Pri-1 Pri-3
lowering the severity of this bug.
It turns out that there are 2 different issues that were happening at the same time and make this look like the culprit.

So, this will be nice to fix but does not seem to be needed for current release.


Comment 48 by benhenry@google.com, Jan 16 (6 days ago)

Components: Test>Telemetry

Comment 49 by benhenry@google.com, Jan 16 (6 days ago)

Components: -Tests>Telemetry

Sign in to add a comment