Skip to content

Conversation

@mre
Copy link
Member

@mre mre commented Oct 30, 2022

Still some styling issues. 😕

image

@Ghadyk Ghadyk changed the title WIP: Work on sponsor page Work on sponsor page Nov 4, 2022
@Ghadyk
Copy link
Collaborator

Ghadyk commented Nov 4, 2022

Something is broken in the deploy action, the error that comes up has already been fixed and the build step works fine locally but for some reason it keeps failing here like it is not pulling the latest version

@mre
Copy link
Member Author

mre commented Nov 4, 2022

It doesn't use your commit but always the latest commit from main.

Source:
In the latest run for this branch here I can see

IMAGE_NAME: us-central1-docker.pkg.dev/analysis-tools-dev/analysis-tools/website:51d5bb293c5638646d525b96e6fa0ed49bfe6253

in the "Build docker image" stage, which is the last git_sha from main.
So it doesn't use the proper branch on build.
I'll see what I can do.

@mre
Copy link
Member Author

mre commented Nov 4, 2022

Can you reintegrate main into this branch? I fixed it there I guess.
Was an easy fix. We used pull_request_target, which checks out the main branch.

@mre
Copy link
Member Author

mre commented Nov 4, 2022

It's still failing but for a different reason.
Looks like there's a recursion issue.

@mre
Copy link
Member Author

mre commented Nov 5, 2022

The reason is that we do too many requests to the Github API (403):

Cache data for: screenshots-chart-testing does not exist - calling API
Error occurred:  {"name":"HttpError","status":403,"response":{"url":"https://api.github.com/repos/analysis-tools-dev/assets/contents/screenshots%2Fchart-testing","status":403,"headers":{"access-control-allow-origin":"*","access-control-expose-headers":"ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, X-RateLimit-Used, X-RateLimit-Resource, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, Deprecation, Sunset","connection":"close","content-length":"279","content-security-policy":"default-src 'none'; style-src 'unsafe-inline'","content-type":"application/json; charset=utf-8","date":"Fri, 04 Nov 2022 22:01:12 GMT","referrer-policy":"origin-when-cross-origin, strict-origin-when-cross-origin","server":"Varnish","strict-transport-security":"max-age=31536000; includeSubdomains; preload","x-content-type-options":"nosniff","x-frame-options":"deny","x-github-media-type":"github.v3; format=json","x-github-request-id":"0540:22B9:15C7EBC:1652E28:63658BA8","x-ratelimit-limit":"60","x-ratelimit-remaining":"0","x-ratelimit-reset":"1667602840","x-ratelimit-resource":"core","x-ratelimit-used":"60","x-xss-protection":"1; mode=block"},"data":{"message":"API rate limit exceeded for 20.66.109.224. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)","documentation_url":"https://docs.github.com/rest/overview/resources-in-the-rest-api#rate-limiting"}},"request":{"method":"GET","url":"https://api.github.com/repos/analysis-tools-dev/assets/contents/screenshots%2Fchart-testing","headers":{"accept":"application/vnd.github+json","user-agent":"analysis-tools (https://github.com/analysis-tools-dev) octokit-core.js/3.6.0 Node.js/19.0.0 (linux; x64)"},"request":{}}}
Warning: data for page "/tool/[slug]" (path "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/tool/chart-testing") is 304 kB which exceeds the threshold of 128 kB, this amount of data can reduce performance.
See more info here: https://nextjs.org/docs/messages/large-page-data
Error: Unable to detect a Project Id in the current environment. 

To avoid this we could store the screenshots in a cache or throttle the Github requests. 🤷

@mre
Copy link
Member Author

mre commented Nov 7, 2022

There is a merge conflict, which I don't know how to handle. Other than that looks good to me and is ready to be merged.

@Ghadyk Ghadyk merged commit 5e17b13 into main Nov 9, 2022
@Ghadyk Ghadyk deleted the sponsors-v2 branch November 9, 2022 20:06
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

Successfully merging this pull request may close these issues.

3 participants