Generalize Japan overview into multi-country webcompat dashboard#506
Generalize Japan overview into multi-country webcompat dashboard#506martinbalfanz wants to merge 4 commits intomozilla:mainfrom
Conversation
| host_min_ranks_condition = "MIN(sightline_rank) <= 1000 OR MIN(global_rank) <= 1000" | ||
| pretty_name = "Sightline" | ||
|
|
||
| [japan_1000] |
There was a problem hiding this comment.
I'm not 100% sure that moving stuff in this file is a good idea; I'm not sure we don't rely on the order anywhere.
| [country] | ||
| title = "Country" | ||
| type = "enum" | ||
| value = "{% for key, metric in metrics.items() if metric.country_code %}{% if loop.first %}{{ metric.pretty_name }}{% endif %}{% endfor %}" |
There was a problem hiding this comment.
It's also OK to just not set the value here, then we don't try to overwrite it on deploy.
| WHERE bugs.resolution = "" AND | ||
| CASE "{{ param("country") }}" | ||
| {% for key, metric in metrics.items() if metric.country_code %} | ||
| WHEN "{{ metric.pretty_name }}" THEN bugs.is_{{ key }} |
There was a problem hiding this comment.
| WHEN "{{ metric.pretty_name }}" THEN bugs.is_{{ key }} | |
| WHEN "{{ metric.pretty_name }}" THEN {{ metric.condition("bugs") }} |
| CASE "{{ param("country") }}" | ||
| {% for key, metric in metrics.items() if metric.country_code %} | ||
| WHEN "{{ metric.pretty_name }}" THEN ( | ||
| (bugs.is_{{ key }} AND NOT bugs.is_global_1000) |
There was a problem hiding this comment.
| (bugs.is_{{ key }} AND NOT bugs.is_global_1000) | |
| ({{ metric.condition(bugs) }} AND NOT bugs.is_global_1000) |
jobs/webcompat-kb/docs/metrics.md
Outdated
| (`country_webcompat_overview`) and are meaningful only for country-specific | ||
| metrics: | ||
|
|
||
| * `country_code` - ISO country code string (e.g. `"jp"`, `"us"`). When |
There was a problem hiding this comment.
There is already the dashboards entry you can put on metrics like
[metric]
…
dashboards = ["country_webcompat_overview"]Then you can loop over just the metrics that are defined to be on that dashboard like {% for metric in dashboard_metrics %} […] {% endfor %}
(this is arguably the wrong way around and it's better separation of logic if each dashboard defines which metrics it wants to include, but it does seem likely to be less typing for common cases when we define it like this).
- Restore core/global_5000 to their original position in metrics.toml - Remove value from country parameter - Use metric.condition() in query templates - Switch from country_code filtering to dashboards field; update all templates to use dashboard_metrics - Update docs to reflect that country_code is not a filter
| type = "site_reports_field" | ||
| host_min_ranks_condition = "MIN(japan_rank) <= 1000" | ||
| pretty_name = "Japan" | ||
| country_code = "jp" |
There was a problem hiding this comment.
I think you can remove all of these country_code values and the associated code in the metrics.py file and the documentation, since they're not used.
2 participants
Checklist for reviewer:
referenced, the pull request should include the bug number in the title)
.circleci/config.yml) will cause environment variables (particularlycredentials) to be exposed in test logs
telemetry-airflow
responsibly.
Note for deployments: In order to push images built by this PR, the user who merges the PR
must be in the telemetry Github team.
This is because deploys depend on the
data-eng-airflow-gcr CircleCI context.
See DENG-8850 for additional discussion.