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

Promisify validation #8319

Merged
merged 3 commits into from
Jan 27, 2021
Merged

Promisify validation #8319

merged 3 commits into from
Jan 27, 2021

Conversation

bhousel
Copy link
Member

@bhousel bhousel commented Jan 25, 2021

This would follow #8305

Rewrite the validator in ES6/Promises, several improvements here:

  • implements a validation work queue, jobs are run during browser idle callbacks
  • when merging base entities, don't run validations 2x on both base and head graphs (this was wasteful)
  • keep track of resolved issues in a separate set (it's not a simple compare of base/head anymore)
    This happens after validation queue is empty and avoids race conditions and inaccurate resolved counts

This isn't quite ready yet, but will be soon. My plan is to make each of the validators either

  • return promises themselves that resolve whenever they are ready to do their thing or
  • have some status property so that the validator can just keep them queued until they are ready to do their thing (probably easier).

The issue I'm trying to solve here I mentioned on #8305 - some validators fetch data from elsewhere and can't properly validate until they have everything they need.

- implements a validation work queue, jobs are run during browser idle callbacks
- when merging base entities, don't run validations 2x on both base and head graphs (this was wasteful)
- keep track of resolved issues in a separate set (it's not a simple compare of base/head anymore)
  this happens after validation queue is empty and avoids race conditions and inaccurate resolved counts
@bhousel bhousel changed the base branch from develop to nsi-v5 January 26, 2021 05:03
@bhousel
Copy link
Member Author

bhousel commented Jan 26, 2021

  • have some status property so that the validator can just keep them queued until they are ready to do their thing (probably easier).

I did this 👍 This PR is good now.
Now validators can return a "provisional" result. This is useful for situations like the outdated_tags validator, which may take a few seconds to be ready as it fetches the latest name-suggestion-index data at startup.

When a validation rule returns a provisional result, the validation engine will put that entity back into the validation queue after a short delay.

@kymckay kymckay added enhancement An enhancement to make an existing feature even better validation An issue with the validation or Q/A code labels Jan 26, 2021
@bhousel
Copy link
Member Author

bhousel commented Jan 27, 2021

I'm happy with how this side quest turned out, so I'm going to merge it back to the nsi_v5 branch and continue from there.

The next logical step is to just use web workers for the stuff we're using requestIdleCallback for. I found a decent library for this, so I might do that in the next few days.

@bhousel bhousel merged commit 16f2f07 into nsi-v5 Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement to make an existing feature even better validation An issue with the validation or Q/A code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants