698789 - Cached validation data needs to be cleared for queued Jetpack add-ons

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Add-on Validation, defect)

Description

[Kris Maglione [:kmag]]

Add-ons which were already in the queue when bug 697509 was fixed still have cached validation data, which makes them impossible to review for most reviewers. The validation results for these add-ons should be regenerated.

Comment 1

[Jorge Villalobos [:jorgev] (he/him)]

How many add-ons are we talking about, approximately?

Comment 2

[Kris Maglione [:kmag]]

My guess is somewhere between 60 and 80. It was over 90 at the end of last week.

Comment 3

[Kumar McMillan [:kumar]]

There's no formal process for this (yet) so it would involve a migration script that deletes the validation columns in the DB and re-runs the validator on the existing files.  

It would be nice to give admins a re-validate button next to each addon version as well.

Comment 4

[Jorge Villalobos [:jorgev] (he/him)]

There's no need to rerun the validation, unless the editor tools code assumes that the results are already cached. It used to be the case that opening the validation page for an add-on would run the validation if there were no cached results.

Comment 5

[Wil Clouser [:clouserw]]

I think comment 4 is correct.  Do these results ever expire on their own?  Is there a list of IDs to apply this to?

Comment 6

[Kumar McMillan [:kumar]]

Hmm, yeah what Jorge says did work at one time.  I'm not 100% positive it still works because the validator was in flux back then.  If that's the case (and it's easy to test on -dev) then we can just update all the jetpacks you want fixed like:

    delete from file_validation fv
    join files f on f.id=fv.file_id
    join versions v on v.id=f.version_id
    where v.id in (...) 

We just need a list of version IDs.

Comment 7

[Kris Maglione [:kmag]]

Can we just clear the data for all active Jetpack reviews?

Comment 8

[Kumar McMillan [:kumar]]

Yeah, I think that would be something like:

    delete from file_validation fv
    join files f on f.id=fv.file_id
    where f.jetpack_version is not null and f.status = <unreviewed>

would need testing and qa of course

Comment 10

[Andrew Williamson [:eviljeff]]

(In reply to Jorge Villalobos [:jorgev] from comment #4)
> It used to be the case that
> opening the validation page for an add-on would run the validation if there
> were no cached results.

A while back (under the old validator??) the results expired every 24 hours.  It meant we were always validating against the current checks rather than what happened to be in place when the version was uploaded (which given the current queue lengths could be 40 days prior).  I'd prefer the daily expiry to return.

Comment 11

[Kris Maglione [:kmag]]

I'd prefer the results to only expire after a significant validator update. Some add-ons take ages to validate and put no small strain on my system resources. I don't imagine that the servers would fare much better.

Comment 12

[Wil Clouser [:clouserw]]

There isn't a method for saying whether an update is significant or not (since it may or may not be depending on the add-on).  We can bring back the 24 hour expiry if you like, as I recall we removed it due to complaints.

Comment 13

[Andrew Williamson [:eviljeff]]

Could we expire the results once a week after the push when the validator might have changed?  Admittedly an addon submission with normal duration in the queue would still lead to a new validation on review.

Comment 14

[Jorge Villalobos [:jorgev] (he/him)]

How about creating a script that clears the validation cache, and making it part of the push to prod process?

Comment 15

[Wil Clouser [:clouserw]]

I was thinking of a script that let you delete anything older than $userinput, but calling it from the push to prod process on everything works too.  We'll do that.

Comment 16

[Wil Clouser [:clouserw]]

pull request for this is in

Comment 17

[Wil Clouser [:clouserw]]

this landed.  Will go out thurs

Comment 18

[Jorge Villalobos [:jorgev] (he/him)]

Reclassifying editor bugs and changing to a new whiteboard flag. Spam, spam, spam, spam...