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

Issue 768764 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Last visit > 30 days ago
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Do we really need //tools/perf/chrome_telemetry_build/chromium_config.py ?

Project Member Reported by kraynov@chromium.org, Sep 26 2017

Issue description

Hi Ned,
This config
[1] https://cs.chromium.org/chromium/src/tools/perf/chrome_telemetry_build/chromium_config.py
looks as a hack to configure Telemetry.

Can we achieve the same effect using Telemetry public API or by making it?
Also it was unobvious that //tools/perf/run_benchmark and folks can override default behavoiur of Telemetry.
[2] https://crbug.com/768334

I think it would be better to avoid and prevent overriding configuration which is not intended to be configurable.
Thanks!
 
Telemetry doesn't meant to know about chromium directory (it lives in catapult/ which chromium/src/ depends on, no the other way around), so this find is part of the API that configure Telemetry to know where to look for certain files. 

Is the fact that this class is overridable concerns you?
Ah ok, it makes sense since Telemetry is agnostic to Chromium location.
It's alright to inherit ChromiumConfig from ProjectConfig. Thing still concerns me is that we pass a path to json in unspecified format. Better if ProjectConfig will provide methods like set_flash_plugin_path(), etc. It will exlicitly specify which features can be configured and which can't.
It's not time critical anyway.
Status: Untriaged (was: Assigned)
I share your concern that none of the binaries set in the json file seems to be related to the code that use them in an obvious way.

I will leave this bug around for backlog purpose.
Thanks!
Status: Assigned (was: Untriaged)
Components: Test>Telemetry
Components: -Speed>Telemetry

Sign in to add a comment