Add items and properties counts to PlatformStatsSummaryJob #795
Conversation
Co-authored-by: Frederik Ring <frederik.ring@gmail.com>
app/Traits/PageFetcher.php
Outdated
| //this function is used to fetch pages on namespace | ||
| function fetchPagesInNamespace(string $wikiDomain, int $namespace): array | ||
| { | ||
| $this->apiUrl = getenv('PLATFORM_MW_BACKEND_HOST').'/w/api.php'; |
There was a problem hiding this comment.
Nitpick: I feel the trait reading configuration values all by itself is a bit of "overstepping" its own responsibilities (the trait shouldn't have any idea how it's being configured), so how about:
- setting the
apiUrlprop in the class constructors as before - in this method, raise a
RuntimeExceptionin case it hasn't been set
| @@ -28,17 +30,25 @@ | |||
| */ | |||
| class PlatformStatsSummaryJob extends Job | |||
There was a problem hiding this comment.
This job now probably requires an increased timeout value so that in production it doesn't get killed by the supervisor while still fetching data. Other jobs that poll all wikis currently use a value of 1800 so maybe it's a good idea to use the same one here. E.g.:
api/app/Jobs/PollForMediaWikiJobsJob.php
Line 13 in 5037c4f
app/Jobs/PlatformStatsSummaryJob.php
Outdated
| use App\Helper\MWTimestampHelper; | ||
| use App\Wiki; | ||
| use App\User; | ||
| use Illuminate\Database\DatabaseManager; | ||
| use Illuminate\Support\Facades\Http; |
There was a problem hiding this comment.
If I'm not mistaken this is not used anymore now that the code has been moved to the trait, but it also won't really hurt much.
There was a problem hiding this comment.
oh yes I missed that
|
Would you mind adding a CHANGELOG entry still before merging? No need to get this reviewed though. |
No description provided.