Skip to content

Commit 8e241e0

Browse files
author
Kristian Dalmo Olsen
authored
Add query cost test (#177)
* Add E2E test to track the cost of check queries * Use real examples instead of wrong calculations
1 parent 87fdaed commit 8e241e0

2 files changed

Lines changed: 77 additions & 31 deletions

File tree

‎README.md‎

Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ Make sure to check out [#migrating](#migrating) to learn more.
3030
| `disable_ci_skip` | No | `true` | Disable ability to skip builds with `[ci skip]` and `[skip ci]` in commit message or pull request title. |
3131
| `skip_ssl_verification` | No | `true` | Disable SSL/TLS certificate validation on git and API clients. Use with care! |
3232
| `disable_forks` | No | `true` | Disable triggering of the resource if the pull request's fork repository is different to the configured repository. |
33-
| `required_review_approvals` | No | `2` | Disable triggering of the resource if the pull request does not have at least `X` approved review(s). |
33+
| `required_review_approvals` | No | `2` | Disable triggering of the resource if the pull request does not have at least `X` approved review(s). |
3434
| `git_crypt_key` | No | `AEdJVENSWVBUS0VZAAAAA...` | Base64 encoded git-crypt key. Setting this will unlock / decrypt the repository with git-crypt. To get the key simply execute `git-crypt export-key -- - | base64` in an encrypted repository. |
3535
| `base_branch` | No | `master` | Name of a branch. The pipeline will only trigger on pull requests against the specified branch. |
3636
| `labels` | No | `["bug", "enhancement"]` | The labels on the PR. The pipeline will only trigger on pull requests having at least one of the specified labels. |
@@ -63,12 +63,12 @@ generate notifications over the webhook. So if you have a repository with little
6363

6464
#### `get`
6565

66-
| Parameter | Required | Example | Description |
67-
|-----------------------|----------|----------|------------------------------------------------------------------------------------|
68-
| `skip_download` | No | `true` | Use with `get_params` in a `put` step to do nothing on the implicit get. |
69-
| `integration_tool` | No | `rebase` | The integration tool to use, `merge`, `rebase` or `checkout`. Defaults to `merge`. |
70-
| `git_depth` | No | `1` | Shallow clone the repository using the `--depth` Git option |
71-
| `list_changed_files` | No | `true` | Generate a list of changed files and save alongside metadata |
66+
| Parameter | Required | Example | Description |
67+
|----------------------|----------|----------|------------------------------------------------------------------------------------|
68+
| `skip_download` | No | `true` | Use with `get_params` in a `put` step to do nothing on the implicit get. |
69+
| `integration_tool` | No | `rebase` | The integration tool to use, `merge`, `rebase` or `checkout`. Defaults to `merge`. |
70+
| `git_depth` | No | `1` | Shallow clone the repository using the `--depth` Git option |
71+
| `list_changed_files` | No | `true` | Generate a list of changed files and save alongside metadata |
7272

7373
Clones the base (e.g. `master` branch) at the latest commit, and merges the pull request at the specified commit
7474
into master. This ensures that we are both testing and setting status on the exact commit that was requested in
@@ -106,18 +106,18 @@ empty commit to the PR*.
106106

107107
#### `put`
108108

109-
| Parameter | Required | Example | Description |
110-
|----------------|----------|-------------------------------------|-------------------------------------------------------------------------------------------------------------------|
111-
| `path` | Yes | `pull-request` | The name given to the resource in a GET step. |
112-
| `status` | No | `SUCCESS` | Set a status on a commit. One of `SUCCESS`, `PENDING`, `FAILURE` and `ERROR`. |
113-
| `base_context` | No | `concourse-ci` | Base context (prefix) used for the status context. Defaults to `concourse-ci`. |
114-
| `context` | No | `unit-test` | A context to use for the status, which is prefixed by `base_context`. Defaults to `status`. |
115-
| `comment` | No | `hello world!` | A comment to add to the pull request. |
116-
| `comment_file` | No | `my-output/comment.txt` | Path to file containing a comment to add to the pull request (e.g. output of `terraform plan`). |
117-
| `target_url` | No | `$ATC_EXTERNAL_URL/builds/$BUILD_ID` | The target URL for the status, where users are sent when clicking details (defaults to the Concourse build page). |
118-
| `description` | No | `Concourse CI build failed` | The description status on the specified pull request. |
119-
| `description_file` | No | `my-output/description.txt` | Path to file containing the description status to add to the pull request |
120-
| `delete_previous_comments` | No | `true` | Boolean. Previous comments made on the pull request by this resource will be deleted before making the new comment. Useful for removing outdated information. |
109+
| Parameter | Required | Example | Description |
110+
|----------------------------|----------|--------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------|
111+
| `path` | Yes | `pull-request` | The name given to the resource in a GET step. |
112+
| `status` | No | `SUCCESS` | Set a status on a commit. One of `SUCCESS`, `PENDING`, `FAILURE` and `ERROR`. |
113+
| `base_context` | No | `concourse-ci` | Base context (prefix) used for the status context. Defaults to `concourse-ci`. |
114+
| `context` | No | `unit-test` | A context to use for the status, which is prefixed by `base_context`. Defaults to `status`. |
115+
| `comment` | No | `hello world!` | A comment to add to the pull request. |
116+
| `comment_file` | No | `my-output/comment.txt` | Path to file containing a comment to add to the pull request (e.g. output of `terraform plan`). |
117+
| `target_url` | No | `$ATC_EXTERNAL_URL/builds/$BUILD_ID` | The target URL for the status, where users are sent when clicking details (defaults to the Concourse build page). |
118+
| `description` | No | `Concourse CI build failed` | The description status on the specified pull request. |
119+
| `description_file` | No | `my-output/description.txt` | Path to file containing the description status to add to the pull request |
120+
| `delete_previous_comments` | No | `true` | Boolean. Previous comments made on the pull request by this resource will be deleted before making the new comment. Useful for removing outdated information. |
121121

122122
Note that `comment`, `comment_file` and `target_url` will all expand environment variables, so in the examples above `$ATC_EXTERNAL_URL` will be replaced by the public URL of the Concourse ATCs.
123123
See https://concourse-ci.org/implementing-resource-types.html#resource-metadata for more details about metadata that is available via environment variables.
@@ -179,19 +179,18 @@ jobs:
179179

180180
## Costs
181181

182-
The Github API(s) have a rate limit of 5000 requests per hour (per user). This resource will incur the following costs:
182+
The Github API(s) have a rate limit of 5000 requests per hour (per user). For the V3 API this essentially
183+
translates to 5000 requests, whereas for the V4 API (GraphQL) the calculation is more involved:
184+
https://developer.github.com/v4/guides/resource-limitations/#calculating-a-rate-limit-score-before-running-the-call
183185

184-
- `check`: Minimum 1, max 1 per 100th *open* pull request.
185-
- `in`: Fixed cost of 1. Fetches the pull request at the given commit.
186-
- `out`: Minimum 1, max 3 (1 for each of `status`, `comment` and `comment_file`).
186+
Ref the above, here are some examples of running `check` against large repositories and the cost of doing so:
187+
- [concourse/concourse](https://github.com/concourse/concourse): 51 open pull requests at the time of testing. Cost 2.
188+
- [torvalds/linux](https://github.com/torvalds/linux): 305 open pull requests. Cost 8.
189+
- [kubernetes/kubernetes](https://github.com/kubernetes/kubernetes): 1072 open pull requests. Cost: 22.
187190

188-
E.g., typical use for a repository with 125 open pull requests will incur the following costs for every commit:
189-
190-
- `check`: 2 (paginate 125 PR's with 100 per page)
191-
- `in`: 1 (fetch the pull request at the given commit ref)
192-
- `out`: 1 (set status on the commit)
193-
194-
With a rate limit of 5000 per hour, it could handle 1250 commits between all of the 125 open pull requests in the span of that hour.
191+
For the other two operations the costing is a bit easier:
192+
- `get`: Fixed cost of 1. Fetches the pull request at the given commit.
193+
- `put`: Uses the V3 API and has a min cost of 1, +1 for each of `status`, `comment` and `comment_file` etc.
195194

196195
## Migrating
197196

‎e2e/e2e_test.go‎

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,41 @@ func TestCheckE2E(t *testing.T) {
189189
}
190190
}
191191

192+
func TestCheckAPICostE2E(t *testing.T) {
193+
tests := []struct {
194+
description string
195+
source resource.Source
196+
version resource.Version
197+
expected int
198+
}{
199+
{
200+
description: "check has a known cost against ratelimit",
201+
source: resource.Source{
202+
Repository: "itsdalmo/test-repository",
203+
AccessToken: os.Getenv("GITHUB_ACCESS_TOKEN"),
204+
},
205+
version: resource.Version{},
206+
expected: 2,
207+
},
208+
}
209+
210+
for _, tc := range tests {
211+
t.Run(tc.description, func(t *testing.T) {
212+
githubClient, err := resource.NewGithubClient(&tc.source)
213+
require.NoError(t, err)
214+
215+
before := getRemainingRateLimit(t, githubClient.V4)
216+
217+
input := resource.CheckRequest{Source: tc.source, Version: tc.version}
218+
_, err = resource.Check(input, githubClient)
219+
require.NoError(t, err)
220+
221+
cost := before - getRemainingRateLimit(t, githubClient.V4)
222+
assert.Equal(t, tc.expected, cost, "unexpected cost for check")
223+
})
224+
}
225+
}
226+
192227
func TestGetAndPutE2E(t *testing.T) {
193228
tests := []struct {
194229
description string
@@ -436,7 +471,7 @@ func TestGetAndPutE2E(t *testing.T) {
436471
}
437472

438473
func TestPutCommentsE2E(t *testing.T) {
439-
owner := "rcoy-v"
474+
owner := "itsdalmo"
440475
repo := "github-pr-resource-e2e"
441476

442477
tests := []struct {
@@ -585,3 +620,15 @@ func readTestFile(t *testing.T, path string) string {
585620
}
586621
return string(b)
587622
}
623+
624+
func getRemainingRateLimit(t *testing.T, c *githubv4.Client) int {
625+
var query struct {
626+
RateLimit struct {
627+
Remaining int
628+
}
629+
}
630+
if err := c.Query(context.TODO(), &query, nil); err != nil {
631+
t.Fatalf("rate limit query: %s", err)
632+
}
633+
return query.RateLimit.Remaining
634+
}

0 commit comments

Comments
 (0)