Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kill the max-count parameter #315

Open
Hexcles opened this issue Jun 22, 2018 · 6 comments
Open

Kill the max-count parameter #315

Hexcles opened this issue Jun 22, 2018 · 6 comments

Comments

@Hexcles
Copy link
Member

Hexcles commented Jun 22, 2018

Currently the max-count parameter is a bit confusing (or sometimes buggy). The document says:

max-count : Maximum number of runs to get (for each browser).

And we have a default max-count of 1 (not documented).

Interaction with labels=experimental

/api/runs?labels=experimental only returns one run in total, not one run each browser. Other labels seem to work fine, so this is probably related to our special casing the "experimental" label (#311 ).

Interaction with sha=latest

When max-count is specified, sha=latest is ignored. e.g. /api/runs?max-count=5 gives five runs per browser, even though some of the runs are not at the latest SHA.

There might be some other edge cases.

@Hexcles
Copy link
Member Author

Hexcles commented Jun 22, 2018

I just confirmed that the first issue would be fixed by my #312 : https://remove-experimental-label-sp-dot-wptdashboard-staging.appspot.com/api/runs?labels=experimental

@Hexcles Hexcles changed the title Sort out the max-count param in various APIs Kill the max-count parameter Jul 12, 2018
@Hexcles
Copy link
Member Author

Hexcles commented Jul 12, 2018

After some offline discussion, we finally realized that max-count was originally created for the /test-runs view when we didn't have the from parameter.

It turned out max-count wasn't ideal for that view, either (see #3 ), so we implemented from.

Basically, there's no use case for max-count any more and we can now safely remove this parameter altogether (regardless of whether we think its original semantics make sense or not).

cc @mdittmer

@lukebjerring
Copy link
Contributor

I suspect @jugglinmike has a use-case for max-count, as he filed bug(s) about it recently?

@jugglinmike
Copy link
Contributor

Thanks, @lukebjerring! (The issues Luke is referencing are gh-569 and gh-570.)

For my part, I'm trying to retrieve the "run" information for all the results uploaded in a given date range (specifically, quarter 3 of 2018). If the API returned all data following the date specified by from, then I could do this with a single request. Though in the absence of a corresponding until parameter (or similar), this could involve a lot of waste (perhaps a prohibitive amount) in some cases.

@jugglinmike
Copy link
Contributor

Also note that currently, the system has limitations on the number of runs it can successfully serve. Without the ability to limit the number of results (e.g. via max-count), data for runs which occurred sufficiently far in the past cannot be retrieved:

$ curl https://wpt.fyi/api/runs?from=2018-07-01T00:00:00Z
API error 1 (datastore_v3: BAD_REQUEST): cannot get more than 1000 keys in a single call

I need this data to determine how dependable the results collection system was over the course of quarter 3.

@foolip discusses this use case in gh-588, and although he doesn't reference any errors, I believe they are the motivation for his request.

@foolip
Copy link
Member

foolip commented Oct 1, 2018

That's the error, yes. I reported it in #587 but that was a dupe.

@lukebjerring lukebjerring removed their assignment Oct 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants