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

ci(linkcheck): use github action instead of downloading tar, and ignore connection failures for cron-based workflow #1676

Merged
merged 4 commits into from
Feb 6, 2023

Conversation

pepopowitz
Copy link
Collaborator

What is the purpose of the change

  1. Swaps a tarball download of https://github.com/filiph/linkcheck for its github action
    I noticed some comments in the actions that indicate we were waiting for a specific release to switch back to the github action. That release has been shipped, so we should switch back to the github action to simplify the workflow definitions.
  2. On the cron-based link-check workflow, reports connection failures as warnings instead of errors
    Our cron-based link-check workflow fails more often than it succeeds, and it appears to be mostly a result of "connection failures" for certain pages. Since we just ignore those failing workflows anyway, we can tell the linkcheck tool not to bother reporting them as errors, and hopefully get to more successful runs than failed ones.

PR Checklist

  • My changes apply to an already released version, and I have added them to the relevant /versioned_docs directory, or they are not for an already released version.
  • My changes apply to future versions, and I have added them to the main /docs directory, or they are not for future versions.
  • My changes require an Engineering review, and I've assigned an engineering manager or tech lead as a reviewer, or my changes do not require an Engineering review.
  • My changes require a technical writer review, and I've assigned @christinaausley as a reviewer, or my changes do not require a technical writer review.

@pepopowitz pepopowitz requested a review from akeller February 3, 2023 22:36
@pepopowitz pepopowitz self-assigned this Feb 3, 2023
@pepopowitz pepopowitz added the hold This issue is parked, do not merge. label Feb 3, 2023
@pepopowitz
Copy link
Collaborator Author

Well......I'm not sure that flag does what it claims to do.

This workflow was run with that flag enabled. It appears to have failed on a connection error!!!

This workflow proves to myself that the way I passed the argument was correct. This one is failing because I ran it with garbage instead of that flag, and it failed on not recognizing the option.

I ran the workflow with the correct flag again and I think it's going to succeed....but note that neither step outputs any warnings (yet -- I'm signing off before it finishes.). I'd expect to see warnings if it had any connection failures.

I'm not sure what to do with this one. At the very least the flag doesn't hurt anything.....it just might not actually do anything.

@pepopowitz pepopowitz added dx Documentation infrastructure typically handled by the Camunda DX team and removed hold This issue is parked, do not merge. labels Feb 6, 2023
@pepopowitz
Copy link
Collaborator Author

Well......I'm not sure that flag does what it claims to do.

....

I'm not sure what to do with this one. At the very least the flag doesn't hurt anything.....it just might not actually do anything.

Having thought about this more, I'd like to proceed with reviewing/merging this PR. It doesn't hurt anything. It might make the link-checking succeed more frequently. I'd like to get it out there, and see what the effect is with a larger sample size.

@pepopowitz
Copy link
Collaborator Author

More notes:

1. Here's another failed workflow, where all that appears in the output are warnings.
2. I opened this issue with the linkcheck project.
3. I suspect the problem might be that when there are warnings it's still spitting out a bunch of messages, but I don't know enough about dart & github actions to fiddle with it and find out.

Copy link
Member

@akeller akeller left a comment

Choose a reason for hiding this comment

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

No issues with this. Read through your thoughts and it sounds like this is the preferred way we want to go and it doesn't seem to break more than was already broken.

@pepopowitz pepopowitz merged commit 82a204d into main Feb 6, 2023
@pepopowitz pepopowitz deleted the pepopowitz/warn-link-check-connection-errors branch February 6, 2023 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dx Documentation infrastructure typically handled by the Camunda DX team
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants