Skip to content

fix(api): make PollForMediaWikiJobsJob skip soft-deleted wikis - #1221

Open
dati18 wants to merge 8 commits into
mainfrom
fix-pending-jobs
Open

fix(api): make PollForMediaWikiJobsJob skip soft-deleted wikis#1221
dati18 wants to merge 8 commits into
mainfrom
fix-pending-jobs

Conversation

@dati18

@dati18 dati18 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Bug: T433575

@dati18
dati18 force-pushed the fix-pending-jobs branch from 72483ae to 3a7351f Compare July 30, 2026 15:19
Comment thread app/Jobs/PollForMediaWikiJobsJob.php Outdated
Comment thread app/Jobs/PollForMediaWikiJobsJob.php Outdated
@dati18
dati18 force-pushed the fix-pending-jobs branch from 048f952 to 668af07 Compare July 30, 2026 15:40
@dati18
dati18 force-pushed the fix-pending-jobs branch from 2bc4c02 to 847d264 Compare August 3, 2026 11:08
@dati18
dati18 force-pushed the fix-pending-jobs branch from 847d264 to 4904673 Compare August 3, 2026 11:09
Comment thread app/Services/MediaWikiHostResolver.php Outdated

@tarrow tarrow left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks close to ready to me apart from the new duplication in the resolver; I wonder if we need to any more tests or do you think that's not necessary?

@tarrow

tarrow commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Thanks for removing the duplication

I wonder if we need to any more tests or do you think that's not necessary?

Did you have any thoughts about this?

@dati18

dati18 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

@tarrow Sorry I couldn't reply sooner because of the outage yesterday. My 2cent is that the extra test would be testing getHostForDomain() returning all expected service hosts using the same helper function, so it's not about testing a behavior change, but a formatting test.
All existing focused tests cover the real contract of MediaWikiHostResolver.php, for example, successful backend host resolution or backend URL generation, etc. That is enough because the change was only about the host-formatting logic, not changing the mapping behavior itself.

@tarrow

tarrow commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

We talked about this in the daily @dati18 thinks that no more testing is required as per the above comment.

I'm still somewhat hesitant to approve this without doing some more thinking / manual testing to improve my confidence; I don't want us to get distracted from our main work if this Job running blows up (although by eye I don't see why it should)

Specifically I think it might be good to have tests that:

  • cover any new public functions added to MediaWikiHostResolver
  • cover the functionality of PollForMediaWikiJobsJob when errors are thrown by MediaWikiHostResolver

I didn't yet manage to find the time for some manual testing or to write these tests myself to improve my confidence.

@dati18

dati18 commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

@tarrow I took your hints and added a few test cases based on existing tests.

MediaWikiHostResolverTest:

  • testResolverReturnsAllHostsForDomain — verifies getHostsForDomain() returns all service hosts with correct names
    testResolverBuildsBackendUrlForWiki — verifies getBackendUrlForWiki() builds the correct http:// URL when given a Wiki object.

PollForMediaWikiJobsJobTest:

  • testSkipsWikiWhenResolverThrowsAndContinuesProcessing — verifies that when MediaWikiHostResolver throws UnknownDBVersionException for one wiki, the job logs a warning and skips it, without failing the whole job, then continues dispatching ProcessMediaWikiJobsJob for the remaining healthy wiki(s).

@tarrow tarrow left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good to me; let's make sure when we ship this to staging we ensure we properly try it though.

Make sure there are some jobs created on Wikis and that they run before we promote it to production.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants