)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"16b26deea2751f5cec9af202e052d83416344416","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"ebff06dd_faaba5bf","updated":"2023-09-14 16:24:26.000000000","message":"Patchset 9 was just a rebase to address the merge conflict (thought C git had no problem with it)","commit_id":"eb96abd7b91d6ffeb1833d8bb1d3e9d69da12ea7"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"cb2e8eaf31d4d602a67f42c6c40b98c105ca63ad","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"3ae45546_516ae1a0","updated":"2023-09-14 18:46:40.000000000","message":"recheck this failed in tests.unit.test_v3.TestEarlyFailure and I don\u0027t think that is affected by changes to the github driver so should be unrelated. Additionally it failed because there were jobs still queued and this change should affect everything prior to jobs queuing.","commit_id":"1c6ff3efb4c57c491fbb27d663600495b7d7ef4e"},{"author":{"_account_id":4146,"name":"Clark Boylan","email":"cboylan@sapwetik.org","username":"cboylan"},"change_message_id":"800d8f7fe77fec36324cb8a7685e2ba0050332cb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"0016d599_f5ed2343","updated":"2023-09-14 20:55:08.000000000","message":"recheck was that node just really slow?","commit_id":"1c6ff3efb4c57c491fbb27d663600495b7d7ef4e"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"15079daef3bdeff496e8594e8d17c1a81e139a3f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":15,"id":"7b8603ee_2d766f8a","updated":"2023-09-18 22:42:25.000000000","message":"if tobias has no further thoughts lgtm!","commit_id":"3c2e518c5266c90884414af78bc52bf6dd2e6a39"}],"doc/source/drivers/github.rst":[{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"9b0f78bf708ad07d5e807d05e2544f89a250095e","unresolved":true,"context_lines":[{"line_number":86,"context_line":"  \u003chttps://help.github.com/articles/creating-an-access-token-for-command-line-use/\u003e`_)."},{"line_number":87,"context_line":"  This api_token is not strictly required as Zuul will fallback to"},{"line_number":88,"context_line":"  anonymous access; however, anonymous access is severely limited by"},{"line_number":89,"context_line":"  API rate limits."},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"Then in the zuul.conf, set `webhook_token`, `app_id`, `app_key` and"},{"line_number":92,"context_line":"optionally `api_token`.  After restarting zuul-scheduler, verify in"}],"source_content_type":"text/x-rst","patch_set":11,"id":"7ac22e8a_af9eafb5","line":89,"updated":"2023-09-14 22:04:23.000000000","message":"The original story this fixes was describing a situation where opendev system-config was trying to depends-on: a change in github.com/ansible/ansible and this failed to enqueue in the check queue because a graphql query only works with an authenticated call.\n\nObviously this was seen a long time ago, so things might have changed.  But I don\u0027t think the authentication requirements for the graphql query on github have (https://docs.github.com/en/graphql/guides/forming-calls-with-graphql#authenticating-with-graphql).\n\nThe graphql call in _updateCanMergeInfo() @ https://opendev.org/zuul/zuul/src/branch/master/zuul/driver/github/githubconnection.py#L1744 is still \"unprotected\" in that it doesn\u0027t check you are authenticated.\n\nI think the contention is that this function should not be called for changes enqueued for a check (because they don\u0027t merge) -- but this doesn\u0027t match what what was seen originally in https://review.opendev.org/c/opendev/system-config/+/794353 where checks never started.  \n\nIf there has been a fix for this -- I took a look but am the first to admit I\u0027ve probably missed it -- I think we should explicitly call it out in the CL here as closing the story.  And we probably need to clearly explain the caveat that you need a token to merge changes, but not to check changes... (TBH I\u0027m still fuzzy on how it works in this situation -- I think Zuul would say \"can\u0027t gate this because I can\u0027t merge to the Depends-On: project\"?)\n\nIf there hasn\u0027t been a fix for this, then I\u0027m not sure this is correct.  It is strictly required to have authentication to Depends-On: a project you\u0027re not installed in or you just hit the same 401 in the graphql query as in the story?\n\nI\u0027m super hesitant to -1 this, but I think that no matter how I\u0027m wrong here there is something we can update -- either the CL to explain how the closed story is not longer affected by this, or the description of how the authentication has to work for graphql queries...","commit_id":"1c6ff3efb4c57c491fbb27d663600495b7d7ef4e"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"3a141e5e08d1ab23210dac2a48092d82500854ef","unresolved":true,"context_lines":[{"line_number":86,"context_line":"  \u003chttps://help.github.com/articles/creating-an-access-token-for-command-line-use/\u003e`_)."},{"line_number":87,"context_line":"  This api_token is not strictly required as Zuul will fallback to"},{"line_number":88,"context_line":"  anonymous access; however, anonymous access is severely limited by"},{"line_number":89,"context_line":"  API rate limits."},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"Then in the zuul.conf, set `webhook_token`, `app_id`, `app_key` and"},{"line_number":92,"context_line":"optionally `api_token`.  After restarting zuul-scheduler, verify in"}],"source_content_type":"text/x-rst","patch_set":11,"id":"f4614051_af975b5e","line":89,"in_reply_to":"1505dfb2_0c6ff884","updated":"2023-09-14 23:26:09.000000000","message":"++ to this split up being made quite clear.  perhaps adding little generic examples helps explain too.\n\nAs mentioned in matrix; the system-config example is probably apt.  We have ansible/ara as required projects as we install from their main/master in the devel job, and zuul pulls that for us.  that is ok unauthenticated.\n\nhowever, if you want to depend-on, then that hits the graphql backend to get details on the PR and requires an auth token.\n\nobviously to add comments/push it needs the full access.","commit_id":"1c6ff3efb4c57c491fbb27d663600495b7d7ef4e"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"284c9eb2f3e56d0819d1f0b75ed996e34b2de93a","unresolved":true,"context_lines":[{"line_number":86,"context_line":"  \u003chttps://help.github.com/articles/creating-an-access-token-for-command-line-use/\u003e`_)."},{"line_number":87,"context_line":"  This api_token is not strictly required as Zuul will fallback to"},{"line_number":88,"context_line":"  anonymous access; however, anonymous access is severely limited by"},{"line_number":89,"context_line":"  API rate limits."},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"Then in the zuul.conf, set `webhook_token`, `app_id`, `app_key` and"},{"line_number":92,"context_line":"optionally `api_token`.  After restarting zuul-scheduler, verify in"}],"source_content_type":"text/x-rst","patch_set":11,"id":"1505dfb2_0c6ff884","line":89,"in_reply_to":"7ac22e8a_af9eafb5","updated":"2023-09-14 22:38:29.000000000","message":"I believe you are correct and this can run in check.  So updating the docs to indicate that some kind of authentication (app install or token) is required for any interaction with pull requests sounds good.\n\nBut it\u0027s not requires for use in required projects.  In short:\n\nReporting: authentication with write access required\nDepends-On: authentication required with no special access\nrequired-projects: anonymous okay","commit_id":"1c6ff3efb4c57c491fbb27d663600495b7d7ef4e"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"1de3d65ab6d1b2a2dacb2fa39f86079c5393a8cc","unresolved":false,"context_lines":[{"line_number":95,"context_line":""},{"line_number":96,"context_line":"* Finally, GitHub enforces API rate limits. Authenticated requests get"},{"line_number":97,"context_line":"  significantly larger rate limits. You may want to configure an application"},{"line_number":98,"context_line":"  and api_token to ensure maximum rate limit values."},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"Then in the zuul.conf, set `webhook_token`, `app_id`, `app_key` and"},{"line_number":101,"context_line":"optionally `api_token`.  After restarting zuul-scheduler, verify in"}],"source_content_type":"text/x-rst","patch_set":12,"id":"8f046c2a_679d897d","line":98,"updated":"2023-09-15 16:16:59.000000000","message":"This is supposed to be a step-by-step list of actions to take to set up an application.  We shouldn\u0027t have anything in here more complex than\n* do this\n* if this, then that\n\nI think if we want to go into a bunch of detail about the possible choices people might make, that should be done in the narrative section above, then refer to the choices here.","commit_id":"c171bf4c897610d12e8a65b9e662b29c6bcefd55"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"eeb698c85454b42e21502b35e1aa77187ad1eda2","unresolved":true,"context_lines":[{"line_number":38,"context_line":""},{"line_number":39,"context_line":"  * Reporting: Authentication with write access is required"},{"line_number":40,"context_line":"  * Enqueuing (including Depends-On): Authentication with read access is required"},{"line_number":41,"context_line":"  * `required-projects` listing in jobs: No authentication necessary (for public repos)"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"There are two different ways Zuul can Authenticate its requests to"},{"line_number":44,"context_line":"GitHub. The first is the `api_token`. This `api_token` is used by the"}],"source_content_type":"text/x-rst","patch_set":14,"id":"3e29970d_b197cec0","line":41,"updated":"2023-09-18 00:27:43.000000000","message":"I think this might be a big-enough \"gotcha\" to warrant a bit more expansion in the description here.\n\n* Reporting: Requires authentication with write access to the project so that comments can be left\n\n* Enqueing a merge request (including Depends-On: of a merge request) : the API queries needed to examine MR\u0027s so that Zuul can enqueue them requires authentication with read access.\n\n* `required-projects` listing : no authentication required.  For example, you may have a project where you are only interested in testing against a specific branch of a GitHub project.  In this case you do not need any authentication to have Zuul pull the project.  However, note that if you will ever need to speculatively test a MR in this project, you will require authenticated read access (see note above)","commit_id":"05683fb31806ba18b321e580cd2b0dd694d86ff2"}],"releasenotes/notes/github-api-token-6070169c0d7dd1e2.yaml":[{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"53d1bc0e5a5442897bd858067974d040fa36a161","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"upgrade:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    If using the GitHub driver in app mode, you should also"},{"line_number":5,"context_line":"    generate and add an ``api_token``.  This is required to"},{"line_number":6,"context_line":"    perform GraphQL queries on GitHub for projects the app is"},{"line_number":7,"context_line":"    not installed in."}],"source_content_type":"text/x-yaml","patch_set":8,"id":"4fde8123_715f2ec5","line":4,"updated":"2021-06-07 10:59:34.000000000","message":"I think \u0027should\u0027 is too strong here since this only affects the use case where we had anonymous access to projects. I think we should emphasize that this is optional and only required to access projects that are not under control of zuul.","commit_id":"c1c30c43d7099eea6fec7b5683e97b347661eb1f"}],"tests/fixtures/zuul-push-reqs.conf":[{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"53d1bc0e5a5442897bd858067974d040fa36a161","unresolved":true,"context_lines":[{"line_number":15,"context_line":"[connection github]"},{"line_number":16,"context_line":"driver\u003dgithub"},{"line_number":17,"context_line":"webhook_token\u003d00000000000000000000000000000000000000000"},{"line_number":18,"context_line":"api_token\u003dghp_51abcFzcvf3GxOJpPFUKxsT6JIL3Nnbf39E"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"[connection gerrit]"},{"line_number":21,"context_line":"driver\u003dgerrit"}],"source_content_type":"text/plain","patch_set":8,"id":"2292d9bb_bdf8d537","line":18,"updated":"2021-06-07 10:59:34.000000000","message":"Is this added to every test github connection in the test fixtures? Since the api_token must stay optional (and many deployments won\u0027t add an api token) this should be added to only a few of them (or a specific one to test the fallback).","commit_id":"c1c30c43d7099eea6fec7b5683e97b347661eb1f"}],"zuul/driver/github/githubconnection.py":[{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"53d1bc0e5a5442897bd858067974d040fa36a161","unresolved":true,"context_lines":[{"line_number":985,"context_line":"                                                 inst_id\u003dinst_id,"},{"line_number":986,"context_line":"                                                 reprime\u003dFalse)"},{"line_number":987,"context_line":""},{"line_number":988,"context_line":"            self.log.info(\"No installation ID available for project %s\","},{"line_number":989,"context_line":"                          project_name)"},{"line_number":990,"context_line":"            return \u0027\u0027"},{"line_number":991,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3e3c6716_7496a1df","line":988,"updated":"2021-06-07 10:59:34.000000000","message":"Actually if we have a deployment that is (intentionally) not using an api token this is indeed an error that should be logged as error. I think we may want to switch based on the config here.","commit_id":"c1c30c43d7099eea6fec7b5683e97b347661eb1f"},{"author":{"_account_id":1,"name":"James E. Blair","email":"jim@acmegating.com","username":"corvus"},"change_message_id":"b4e2a8927d583a034f0251c056dc445ad50eeff0","unresolved":false,"context_lines":[{"line_number":985,"context_line":"                                                 inst_id\u003dinst_id,"},{"line_number":986,"context_line":"                                                 reprime\u003dFalse)"},{"line_number":987,"context_line":""},{"line_number":988,"context_line":"            self.log.info(\"No installation ID available for project %s\","},{"line_number":989,"context_line":"                          project_name)"},{"line_number":990,"context_line":"            return \u0027\u0027"},{"line_number":991,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"854229d3_856ee61c","line":988,"updated":"2021-06-07 13:38:17.000000000","message":"Let\u0027s not add a config option for that; we try to keep their number to a minmum.  Instead, let\u0027s log this as error if there is no token, and info if there is.","commit_id":"c1c30c43d7099eea6fec7b5683e97b347661eb1f"},{"author":{"_account_id":7118,"name":"Ian Wienand","email":"iwienand@redhat.com","username":"iwienand"},"change_message_id":"780caf26bf158390f377e4b01a2dea410fe64c9c","unresolved":true,"context_lines":[{"line_number":985,"context_line":"                                                 inst_id\u003dinst_id,"},{"line_number":986,"context_line":"                                                 reprime\u003dFalse)"},{"line_number":987,"context_line":""},{"line_number":988,"context_line":"            self.log.info(\"No installation ID available for project %s\","},{"line_number":989,"context_line":"                          project_name)"},{"line_number":990,"context_line":"            return \u0027\u0027"},{"line_number":991,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"b4272d20_d61bcab0","line":988,"in_reply_to":"3e3c6716_7496a1df","updated":"2021-06-07 12:48:35.000000000","message":"So are you suggesting we add something like \n\n allow_unmamaged_projects\u003dTrue/False\n\nIn the driver config?  If that is true and we don\u0027t have and api key we raise the failure, if False, then not being installed is also an error and we have to fail if there\u0027s no installation is?","commit_id":"c1c30c43d7099eea6fec7b5683e97b347661eb1f"},{"author":{"_account_id":16068,"name":"Tobias Henkel","email":"tobias.henkel@bmw.de","username":"tobias.henkel"},"change_message_id":"3fbc63f3f4f964fe793e247c2be4f16817b479ce","unresolved":true,"context_lines":[{"line_number":985,"context_line":"                                                 inst_id\u003dinst_id,"},{"line_number":986,"context_line":"                                                 reprime\u003dFalse)"},{"line_number":987,"context_line":""},{"line_number":988,"context_line":"            self.log.info(\"No installation ID available for project %s\","},{"line_number":989,"context_line":"                          project_name)"},{"line_number":990,"context_line":"            return \u0027\u0027"},{"line_number":991,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"afcc14a4_ab57e134","line":988,"in_reply_to":"854229d3_856ee61c","updated":"2021-06-08 06:15:04.000000000","message":"Yeah, that\u0027s what I meant, info if there is a token, error if not.","commit_id":"c1c30c43d7099eea6fec7b5683e97b347661eb1f"}]}
