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

Add per-entry ignore of NS validation failures in CI testing #7239

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

makyen
Copy link
Contributor

@makyen makyen commented Sep 3, 2022

This adds the ability to ignore NS validation failures during CI testing on a per-entry basis in the *nses.yml files. If an entry in those files has the value ignore_validation_errors: true, then validation errors will not cause CI failures.

This is primarily in order to be able to prevent CI failures due to the persistent intermittent issues with validating dns-parking.com.. This PR adds ignore_validation_errors: true to the dns-parking.com. entry in watched_nses.yml, which will result in not seeing CI testing failures when that domain fails DNS validation.

There probably should be some automated testing which still validates, and fails on, entries which have ignore_validation_errors: true, as we do want to know about such domains if they actually stop having a DNS entry. However, this PR just patches over the persistent frustration of looking at CI failures and finding that it's yet another time that dns-parking.com. had an intermittent problem. No attempt is made here to address the issue of wanting some type reporting for such domains when they actually go away.

@makyen makyen requested a review from tripleee September 4, 2022 02:05
Copy link
Member

@tripleee tripleee left a comment

Choose a reason for hiding this comment

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

Updating the schema should also update the Schema_version at the top of the YAML file.

I never got around to properly formalizing this, but the purpose of this information is to track changes in the schema, so let's apply it here.

@tripleee
Copy link
Member

tripleee commented Sep 4, 2022

In theory, I dislike the idea of bypassing quality checks when the quality sucks, but the DNS problems of our CI is beyond our control, so I guess let's do what we can to focus on the things we can affect. +1

@stale stale bot added the status: stale label Oct 30, 2022
@stale
Copy link

stale bot commented Nov 12, 2022

This issue has been closed because it has had no recent activity. If this is still important, please add another comment and find someone with write permissions to reopen the issue. Thank you for your contributions.

@stale stale bot closed this Nov 12, 2022
@tripleee tripleee reopened this Nov 12, 2022
@tripleee
Copy link
Member

I guess we still want this, even though the problem has been somewhat mitigated by other means. Sorry for being slow to review.

@tripleee tripleee added status: confirmed Confirmed as something that needs working on. and removed status: stale labels Nov 12, 2022
@makyen makyen closed this May 7, 2023
@makyen makyen deleted the Mak-exclude-some-NS-failures-in-CI branch May 7, 2023 05:42
@makyen makyen restored the Mak-exclude-some-NS-failures-in-CI branch May 7, 2023 05:46
@makyen makyen reopened this May 7, 2023
makyen added 2 commits May 6, 2023 22:55
This adds the ability to ignore NS validation failures during CI
testing on a per-entry basis in the nses.yml files. If an entry
has the value `ignore_validation_errors: true`, then validation
errors will not cause CI failures.

This is primarily in order to be able to prevent CI failures
due to the persistent intermittent issues with validating
`dns-parking.com.`.
@makyen makyen force-pushed the Mak-exclude-some-NS-failures-in-CI branch from 6577f88 to 60a3736 Compare May 7, 2023 06:07
@makyen
Copy link
Contributor Author

makyen commented May 7, 2023

While I do intend to finish this at some point (i.e. integrate the requested changes for which I have some work along those lines), I wasn't intending to work on this at the moment. The activity today was because I managed to mistakenly delete the branch on GitHub, which automatically closed the PR. Restoring the branch on GitHub and reopening the PR re-ran CI, which, of course, had failures, due to being 13k commits behind. So, the work today was just to rebase the already pushed commits onto the current master branch.

@tripleee
Copy link
Member

For the record, the requested change simply requires the Schema_version: key at the top of the file to get a new value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: confirmed Confirmed as something that needs working on.
Development

Successfully merging this pull request may close these issues.

2 participants