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

Upgrade github-script #6920

Merged
merged 4 commits into from
Nov 8, 2024
Merged

Upgrade github-script #6920

merged 4 commits into from
Nov 8, 2024

Conversation

KyleJu
Copy link
Contributor

@KyleJu KyleJu commented Nov 8, 2024

Relate to #6917. Upgrade Artifact actions v4. v3 will be deprecated by December 5, 2024

@KyleJu KyleJu added the do not merge yet Disable auto-merge label Nov 8, 2024
@KyleJu KyleJu requested review from a team as code owners November 8, 2024 19:06
@jcscottiii
Copy link
Collaborator

One thing I see that's different in v7 is the fact that it uses github.rest. instead github. for the prefix. That may affect this too.

@KyleJu
Copy link
Contributor Author

KyleJu commented Nov 8, 2024

Similar error as before:

RequestError [HttpError]: <?xml version="1.0" encoding="utf-8"?>
<Error><Code>AuthenticationFailed</Code><Message>Server failed to authenticate the request. Make sure the value of Authorization header is formed correctly including the signature.
RequestId:362c0930-901e-00e5-3611-[32](https://github.com/web-platform-tests/wpt-metadata/actions/runs/11748079644/job/32731390035#step:2:33)d916000000
Time:2024-11-08T19:08:16.9222017Z</Message></Error>
    at /home/runner/work/_actions/actions/github-script/v3.1.0/dist/index.js:2137:23
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async eval (eval at callAsyncFunction (/home/runner/work/_actions/actions/github-script/v3.1.0/dist/index.js:5368:16), <anonymous>:14:16)
    at async main (/home/runner/work/_actions/actions/github-script/v3.1.0/dist/index.js:5394:20) {
  status: 403,
  headers: {
    'access-control-allow-origin': '*',
    'access-control-expose-headers': 'Content-Length,Content-Type,Date,Server,x-ms-request-id',
    'content-length': '321',
    'content-type': 'application/xml',
    date: 'Fri, 08 Nov 2024 19:08:16 GMT',
    server: 'Windows-Azure-Blob/1.0 Microsoft-HTTPAPI/2.0',
    'x-ms-request-id': '[36](https://github.com/web-platform-tests/wpt-metadata/actions/runs/11748079644/job/32731390035#step:2:37)2c0930-901e-00e5-3611-32d916000000'
  },
  request: {
    method: 'GET',
    url: 'https://api.github.com/repos/web-platform-tests/wpt-metadata/actions/artifacts/2164725286/zip',
    headers: {
      accept: 'application/vnd.github.-preview+json',
      'user-agent': 'actions/github-script octokit-core.js/3.2.1 Node.js/20.13.1 (linux; x64)',
      authorization: 'token [REDACTED]'
    },
    request: { agent: [Agent], hook: [Function: bound bound register] }
  }
}
Error: Unhandled error: HttpError: <?xml version="1.0" encoding="utf-8"?>
<Error><Code>AuthenticationFailed</Code><Message>Server failed to authenticate the request. Make sure the value of Authorization header is formed correctly including the signature.
RequestId:362c0930-901e-00e5-3611-32d916000000
Time:2024-11-08T19:08:16.9222017Z</Message></Error>

@jcscottiii
Copy link
Collaborator

Ignore that one comment. It doesn't even get to that part. Let's keep this PR open. And we can proceed with #6917 for now

@jcscottiii
Copy link
Collaborator

Additionally, you may want to take a look some of these issues:

The first link shows that it is an issue that other are facing when moving to v4

@KyleJu
Copy link
Contributor Author

KyleJu commented Nov 8, 2024

I noticed that the auto-approve is still running on 3.x on. I don't think the change in this PR is reflected in the workflow on master branch

Screenshot 2024-11-08 11 35 56 AM

@KyleJu KyleJu enabled auto-merge (squash) November 8, 2024 19:50
@KyleJu KyleJu merged commit 27369b8 into master Nov 8, 2024
4 checks passed
@KyleJu KyleJu deleted the upgrade-script branch November 8, 2024 19:51
@KyleJu
Copy link
Contributor Author

KyleJu commented Nov 8, 2024

@jcscottiii worked! #6915

@gsnedders
Copy link
Member

Note we shouldn't need the actions/github-script at all, something like https://github.com/web-platform-tests/wpt/blob/e44f4535f4fe673f00e6f487c6240a4c4647b5a6/.github/workflows/check-workflow-run.yml#L32-L37 (note also the permissions: read for the token) using actions/download-artifact instead.

@KyleJu
Copy link
Contributor Author

KyleJu commented Nov 12, 2024

Note we shouldn't need the actions/github-script at all, something like https://github.com/web-platform-tests/wpt/blob/e44f4535f4fe673f00e6f487c6240a4c4647b5a6/.github/workflows/check-workflow-run.yml#L32-L37 (note also the permissions: read for the token) using actions/download-artifact instead.

Filed an issue here,#6935. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge yet Disable auto-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants