Skip to content

Build: disable link-check for existing medium blog posts#10042

Merged
nastra merged 1 commit into
apache:mainfrom
manuzhang:patch-8
Mar 27, 2024
Merged

Build: disable link-check for existing medium blog posts#10042
nastra merged 1 commit into
apache:mainfrom
manuzhang:patch-8

Conversation

@manuzhang
Copy link
Copy Markdown
Member

This fixes #10038

@github-actions github-actions Bot added the docs label Mar 26, 2024
Copy link
Copy Markdown
Contributor

@CsengerG CsengerG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@nastra
Copy link
Copy Markdown
Contributor

nastra commented Mar 26, 2024

While it's great to have retries, it seems that this doesn't solve the issue and the Link checker runs for 48m+. So maybe we should just do "aliveStatusCodes": [429, 200]?

@CsengerG
Copy link
Copy Markdown
Contributor

So maybe we should just do "aliveStatusCodes": [429, 200]?

Is the order of checking the links deterministic? If it is, then that can hide issues (page is not actually present, but by the time you get there it is a 429 --- what are the chances/do we care?). If the order of traversal is not deterministic, then we can perhaps ignore throttling (as you would expect to catch the dead links at some point anyway).

@manuzhang manuzhang changed the title Build: retry on 429 status code in link checker Build: disable link-check for existing medium blog posts Mar 26, 2024
Comment thread site/link-checker-config.json Outdated
],
"retryOn429": true,
"retryCount": 5,
"fallbackRetryDelay": "120s"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we reduce the retries? I think with that retry the last CI run took 48 mins, which is too long IMO

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it will work anyway. I'm not exploring this approach now.

@manuzhang
Copy link
Copy Markdown
Member Author

manuzhang commented Mar 26, 2024

I'm just disabling link check for existing medium blog posts, which have been causing failure due to 429. WDYT?

@nastra
Copy link
Copy Markdown
Contributor

nastra commented Mar 26, 2024

I'm just disabling link check for existing medium blog posts, which have been causing failure due to 429. WDYT?

Yeah I think this is probably the best for now. Can you remove the retry config and we'll go with the current approach?

@manuzhang
Copy link
Copy Markdown
Member Author

Can you remove the retry config and we'll go with the current approach?

Done.

@nastra nastra merged commit fa80c85 into apache:main Mar 27, 2024
@nastra
Copy link
Copy Markdown
Contributor

nastra commented Mar 27, 2024

@manuzhang it looks like there are a few more medium links that haven't been covered. Can you please follow-up on those as well?

@manuzhang manuzhang deleted the patch-8 branch March 27, 2024 08:30
@manuzhang
Copy link
Copy Markdown
Member Author

manuzhang commented Mar 27, 2024

@nastra Sure, do we want to run link check on main branch or just PRs for changed docs?

cc @Fokko @bitsondatadev

@nastra
Copy link
Copy Markdown
Contributor

nastra commented Mar 28, 2024

@manuzhang can you open up a PR to fix the remaining links please? Also IMO we should be running this for PRs and on main

@manuzhang
Copy link
Copy Markdown
Member Author

@nastra just created #10057

sasankpagolu pushed a commit to sasankpagolu/iceberg that referenced this pull request Oct 27, 2024
zachdisc pushed a commit to zachdisc/iceberg that referenced this pull request Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ci: Throttling causes flakyness in link checker

3 participants