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.

Sign up for GitHub

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

Jump to bottom

fix(ci): add cleanup workflow to retain generated-docs-preview for only the last 25 PRs #2758

Merged
merged 5 commits into from
Oct 1, 2024
Merged

fix(ci): add cleanup workflow to retain generated-docs-preview for only the last 25 PRs #2758

merged 5 commits into from
Oct 1, 2024

Conversation

Arqu
Copy link
Collaborator

@Arqu Arqu commented Sep 30, 2024

Description

Breaking Changes

Notes & open questions

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • Tests if relevant.
  • All breaking changes documented.

@Arqu Arqu added the ci continous integration label Sep 30, 2024
@Arqu Arqu self-assigned this Sep 30, 2024
@Arqu Arqu enabled auto-merge September 30, 2024 12:13
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 30, 2024 •
edited
Loading

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/2758/docs/iroh/

Last updated: 2024-10-01T09:09:34Z

@github-actions GitHub Actions
Copy link

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: fa7e42d

@github-actions GitHub Actions
Copy link

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: 3028de9

@Arqu Arqu added this pull request to the merge queue Sep 30, 2024
matheus23
matheus23 requested changes Sep 30, 2024
View reviewed changes
Copy link
Contributor

@matheus23 matheus23 left a comment

Choose a reason for hiding this comment

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

Ehm, I'm not sure if this will work. The generated-docs-preview branch is a "special branch" as far as github is concerned I think.

@matheus23 matheus23 removed this pull request from the merge queue due to a manual request Sep 30, 2024
@matheus23
Copy link
Contributor

matheus23 commented Sep 30, 2024 •
edited
Loading

What exactly is the problem you're trying to solve? Some explanation in the PR description would help me review this!

I've seen the action fail on what I think are race conditions, but I think the way to fix that may be to change the way the branch is updated.
The action should never generate merge conflicts, because you're always pushing into separate directories.

@Arqu
Copy link
Collaborator Author

Arqu commented Sep 30, 2024

Thanks for jumping in, I was hoping it works for any branch you set it to (dynamically).
Anyways, the crux of it is this https://github.com/n0-computer/iroh/actions/runs/11105394334/job/30851460274?pr=2756
It seems mostly to be a race between PRs to publish their own. Maybe do a softer version of and do like up to 5 retries with back off.

@dignifiedquire
Copy link
Contributor

  • what if we switch this to a command instead of auto generating it? eg /generate_docs
  • alternatively, maybe we just push this onto one of our CI machines and run nginx there just hosting these ourselves per branch

@matheus23
Copy link
Contributor

Seems to be a common issue with the action we're using to update the branch: peaceiris/actions-gh-pages#1078

@matheus23
Copy link
Contributor

This was recommended as a fix to the concurrency issue: peaceiris/actions-gh-pages#998 (comment)

I don't think our problem is the concurrency though. It seems the action has stopped at some point in the last month or so.

@matheus23
Copy link
Contributor

Thanks for jumping in, I was hoping it works for any branch you set it to (dynamically). Anyways, the crux of it is this https://github.com/n0-computer/iroh/actions/runs/11105394334/job/30851460274?pr=2756 It seems mostly to be a race between PRs to publish their own. Maybe do a softer version of and do like up to 5 retries with back off.

Yeah actually I think this is a separate issue. If pushing to the generated-docs-preview branch were the problem, then the stuff that's in the branch would still be served correctly, but e.g. the 2760 PR exists on the branch, but isn't served by github for some reason.

@Arqu
Copy link
Collaborator Author

Arqu commented Oct 1, 2024

Thanks for jumping in, I was hoping it works for any branch you set it to (dynamically). Anyways, the crux of it is this https://github.com/n0-computer/iroh/actions/runs/11105394334/job/30851460274?pr=2756 It seems mostly to be a race between PRs to publish their own. Maybe do a softer version of and do like up to 5 retries with back off.

Yeah actually I think this is a separate issue. If pushing to the generated-docs-preview branch were the problem, then the stuff that's in the branch would still be served correctly, but e.g. the 2760 PR exists on the branch, but isn't served by github for some reason.

Yeah thats what I meant by saying it generally wasn't working any more and nothing changed.

As for the initial issue (btw thanks for the research), my plan is as follows:

  • docs are already separate
  • add the concurency group name so we ensure sequential pushing to the branch
  • move back to the original branch
  • then look at fixing the actual preview

@Arqu Arqu force-pushed the arqu/make-docs-preview-unique branch from 644889a to c6fc94d Compare October 1, 2024 07:37
@Arqu Arqu force-pushed the arqu/make-docs-preview-unique branch from c6fc94d to 08a0dd1 Compare October 1, 2024 07:38
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 1, 2024

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: c3721d6

@Arqu
Copy link
Collaborator Author

Arqu commented Oct 1, 2024

Ok, think I've figured it out. Seems like we're hitting some file size limits judging by the action outputs. I'll manually prune some of the old PRs and see if that makes things work again.

@Arqu
Copy link
Collaborator Author

Arqu commented Oct 1, 2024

Yup that worked just fine now. I'll add a step to clean up the branch every now and then.

@Arqu
Copy link
Collaborator Author

Arqu commented Oct 1, 2024

@matheus23 This should now also trigger a weekly job to clean up the docs branch. It does risk to remove the preview for PRs that are old but still open, but I guess if you still need or want the docs preview you can easily re-run the job to generate them.

@Arqu Arqu requested a review from matheus23 October 1, 2024 08:29
matheus23
matheus23 approved these changes Oct 1, 2024
View reviewed changes
Copy link
Contributor

@matheus23 matheus23 left a comment

Choose a reason for hiding this comment

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

Thank you for figuring it out! :)
Just a small nit.

.github/workflows/docs.yaml
@@ -3,6 +3,10 @@ name: Docs Preview
on:
pull_request:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add workflow_dispatch here, too? So we can trigger it manually?

@matheus23 matheus23 changed the title fix(ci): make docs preview unique by PR fix(ci): add cleanup script to retain generated-docs-preview for only the last 25 PRs Oct 1, 2024
@matheus23 matheus23 changed the title fix(ci): add cleanup script to retain generated-docs-preview for only the last 25 PRs fix(ci): add cleanup workflow to retain generated-docs-preview for only the last 25 PRs Oct 1, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 1, 2024

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: 29af8a5

@Arqu
Copy link
Collaborator Author

Arqu commented Oct 1, 2024

Think this should do it, but cant be 100% sure till we merge and test it out due to the nature of workflows and permissions on GHA.

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 1, 2024

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: 47bfbe9

@matheus23
Copy link
Contributor

matheus23 commented Oct 1, 2024 •
edited
Loading

Why does netsim keep adding comments? 😅 (Is that intentional?)

@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 1, 2024

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: cdde4c2

@matheus23 matheus23 added this pull request to the merge queue Oct 1, 2024
@Arqu
Copy link
Collaborator Author

Arqu commented Oct 1, 2024

Why does netsim keep adding comments? 😅 (Is that intentional?)

It's supposed to update them but something is messed up, that's what I'm going to fix next.

Merged via the queue into main with commit 8420674 Oct 1, 2024
27 checks passed
@Arqu Arqu deleted the arqu/make-docs-preview-unique branch October 1, 2024 09:32
matheus23 pushed a commit that referenced this pull request Nov 14, 2024
@Arqu
fix(ci): add cleanup workflow to retain generated-docs-preview for …
1ea0d3d 
…only the last 25 PRs (#2758)

## Description

<!-- A summary of what this pull request achieves and a rough list of
changes. -->

## Breaking Changes

<!-- Optional, if there are any breaking changes document them,
including how to migrate older code. -->

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [ ] Self-review.
- [ ] Documentation updates following the [style
guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text),
if relevant.
- [ ] Tests if relevant.
- [ ] All breaking changes documented.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matheus23 matheus23 matheus23 approved these changes

@dignifiedquire dignifiedquire dignifiedquire approved these changes

Assignees

@Arqu Arqu

Labels
ci continous integration
Projects
iroh
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Arqu @matheus23 @dignifiedquire