mozilla / mozanalysis

A library for Mozilla experiments analysis
https://mozilla.github.io/mozanalysis/
Mozilla Public License 2.0
9 stars 13 forks source link

fix #187: add sample_size param to enrollments query functions #188

Closed mikewilli closed 11 months ago

mikewilli commented 11 months ago

Adds a sample_size parameter to the functions building enrollments queries. This will allow us to update metric-hub configs to support easier downsampling without having to redefine the entire enrollments query. From the config, jetstream can retrieve this sample size and pass it into mozanalysis when building the enrollments query.

codecov-commenter commented 11 months ago

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (93b8644) 75.96% compared to head (00a40d3) 75.96%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #188 +/- ## ======================================= Coverage 75.96% 75.96% ======================================= Files 23 23 Lines 957 957 ======================================= Hits 727 727 Misses 230 230 ``` | [Flag](https://app.codecov.io/gh/mozilla/mozanalysis/pull/188/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mozilla) | Coverage Δ | | |---|---|---| | [py38](https://app.codecov.io/gh/mozilla/mozanalysis/pull/188/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mozilla) | `75.96% <100.00%> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mozilla#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files Changed](https://app.codecov.io/gh/mozilla/mozanalysis/pull/188?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mozilla) | Coverage Δ | | |---|---|---| | [src/mozanalysis/experiment.py](https://app.codecov.io/gh/mozilla/mozanalysis/pull/188?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=mozilla#diff-c3JjL21vemFuYWx5c2lzL2V4cGVyaW1lbnQucHk=) | `78.41% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mikewilli commented 11 months ago

What would an end-to-end test of this look like for jetstream config files? Like, how do we test that when someone sets this parameter in a custom config that the expected enrollments SQL query is generated

Good question. I don't see why we couldn't test for the sample_id <= {value} inside jetstream as it contains mozanalysis as a dependency, with a mock for the object that would've been parsed from the config. Then we can also have separate tests for parsing it from the config.