openvibers/openvibe.tours Commits

Place photos: paced under Media's upload limit; a failed copy is retried (#108)

OP OpenVibers committed 5 days ago · 47acc68f22 · 4 files, +32 −7

* Place photos: paced under Media's upload limit, a failed copy is retried

Media takes 30 uploads a minute from one caller; the photo pass sent them all at once
and most answered 429, and those places were marked tried for a week. Uploads now go
one every 2.1 s, a place whose photo exists but whose copy failed is not marked tried
(photoDeferred), and migration 0011 (migrate) clears the marks left by the 429s.

* Places job version: the first boot retries the refused photos

Changes

ddiff --git a/migrations/0011_retry_place_photos.sql b/migrations/0011_retry_place_photos.sql
nnew file mode 100644
iindex 0000000..90877f7
---- /dev/null
++++ b/migrations/0011_retry_place_photos.sql
@@@ -0,0 +1,4 @@
++-- phase: migrate
++-- On 11-08 Media answered 429 to most photo copies and the photo pass marked those places tried for a week. Clear the
++-- mark on every place still without a photo, so the next runs (now paced under Media's limit) try them again.
++UPDATE tours_places SET photo_tried_at = NULL WHERE image_url IS NULL AND photo_tried_at IS NOT NULL;
ddiff --git a/server/jobs/places.js b/server/jobs/places.js
iindex 4550bbd..7d5103e 100644
---- a/server/jobs/places.js
++++ b/server/jobs/places.js
@@@ -21,7 +21,7 @@ const { isPlace } = require('../places/is-place');
  
  // A run under older rules is not fresh (jobs/index.js): name-en-1 stored OpenStreetMap's English names; wv-text-1
  // rewrites the stored Wikivoyage places' text with the current parser.
--const PLACES_VERSION = 'notable-places-3-photo-pass';   // 11-08: the first boot runs the photo pass at once
++const PLACES_VERSION = 'notable-places-4-photo-pace';   // 11-08: the first boot retries the photos Media refused at once
  
  function commonsFile(value) {
      const raw = String(value || '').trim();
@@@ -110,6 +110,8 @@ function createPlacesJob({ config, s, store, fetchImpl, log = console }) {
                  try { image = await openverse.image(p.name); } catch { /* no verified photo */ }
              }
              const cached = image ? await photoCache.cache(image) : null;
++            // A photo exists but its copy failed (Media busy, a timeout): not "no photo", so it is tried again next run.
++            const deferred = !!image && !cached;
              const gallery = [];
              for (const file of galleries.get(i) || []) {
                  const candidate = images.get(file);
@@@ -120,7 +122,7 @@ function createPlacesJob({ config, s, store, fetchImpl, log = console }) {
              }
              const pictured = cached ? { ...p, imageUrl: cached.url, imageCredit: cached.credit,
                  imageLicense: cached.license, imageLicenseUrl: cached.licenseUrl, imageSourceUrl: cached.sourceUrl, gallery } : p;
--            return { ...pictured, rankScore: Math.max(Number(p.rankScore) || 0,
++            return { ...pictured, ...(deferred ? { photoDeferred: true } : {}), rankScore: Math.max(Number(p.rankScore) || 0,
                  rankPlace(pictured, entities.get(p.wikidataId), pageviews.get(p.wikidataId) || 0)) };
          }));
      }
@@@ -250,7 +252,10 @@ function createPlacesJob({ config, s, store, fetchImpl, log = console }) {
              if (bare.length) {
                  const pictured = await photosFor(bare.map((r) => ({ id: r.id, wikidataId: r.wikidata_id, name: r.name })));
                  result.photos = 0;
--                for (const p of pictured) if (await store.setPlacePhoto(p.id, p)) result.photos += 1;
++                for (const p of pictured) {
++                    if (p.photoDeferred) { result.photosDeferred = (result.photosDeferred || 0) + 1; continue; }
++                    if (await store.setPlacePhoto(p.id, p)) result.photos += 1;
++                }
              }
          } catch (err) {
              result.errors.push(`photos: ${err && err.message}`);
ddiff --git a/server/places/photo-cache.js b/server/places/photo-cache.js
iindex a8ed1c2..38ef499 100644
---- a/server/places/photo-cache.js
++++ b/server/places/photo-cache.js
@@@ -2,8 +2,17 @@
  
  // A checked Commons image is copied once into Media as a public object. The file's Commons page,
  // author and licence stay on the place row; Media serves only the smaller image bytes.
--function createPhotoCache({ config, s, fetchImpl = globalThis.fetch, log = console, objectsClient = null }) {
++// Media takes at most 30 uploads a minute from one caller (media.object.upload); one every UPLOAD_GAP_MS stays under it.
++const UPLOAD_GAP_MS = 2_100;
++function createPhotoCache({ config, s, fetchImpl = globalThis.fetch, log = console, objectsClient = null, uploadGapMs = UPLOAD_GAP_MS }) {
      let objects = objectsClient;
++    let nextUploadAt = 0;
++    async function uploadSlot() {
++        const now = Date.now();
++        const at = Math.max(now, nextUploadAt);
++        nextUploadAt = at + uploadGapMs;
++        if (at > now) await new Promise((ok) => setTimeout(ok, at - now));
++    }
      function client() {
          if (!objects) {
              const { createServiceTokenClient } = require('openvibe-sdk/auth');
@@@ -45,6 +54,7 @@ function createPhotoCache({ config, s, fetchImpl = globalThis.fetch, log = conso
              const bytes = Buffer.from(await res.arrayBuffer());
              if (bytes.length > 4_000_000) return null;
              const file = image.sourceUrl.split('/').pop().slice(0, 100) || 'place-photo.jpg';
++            await uploadSlot();
              const obj = await client().upload(bytes, { kind: 'file', visibility: 'public', mimeType: type, filename: file });
              const url = obj.public_url || obj.url;
              if (!url || !/^https:\/\/openvibe\.media\//.test(url)) { log.warn(`[Tours photos] Media answered no public openvibe.media URL for ${image.sourceUrl}`); return null; }
ddiff --git a/test/photo-cache-widths.test.js b/test/photo-cache-widths.test.js
iindex d3c9344..657669e 100644
---- a/test/photo-cache-widths.test.js
++++ b/test/photo-cache-widths.test.js
@@@ -20,23 +20,29 @@ const objectsClient = { upload: async (bytes) => { uploaded.push(bytes.length);
  (async () => {
      // 1. 960px is asked first and copied.
      let asked = [];
--    let cache = createPhotoCache({ config, s: { db }, objectsClient, log: { warn() {} }, fetchImpl: async (u) => { asked.push(u); return ok(); } });
++    let cache = createPhotoCache({ config, s: { db }, objectsClient, uploadGapMs: 0, log: { warn() {} }, fetchImpl: async (u) => { asked.push(u); return ok(); } });
      let out = await cache.cache(image);
      assert.ok(/\/960px-Kubota\.jpg$/.test(asked[0]), 'a standard Commons width first');
      assert.strictEqual(out.url, 'https://openvibe.media/api/v2/tours/objects/obj_1');
  
      // 2. 960 refused (a small original): the exact imageinfo URL is used.
      asked = [];
--    cache = createPhotoCache({ config, s: { db }, objectsClient, log: { warn() {} }, fetchImpl: async (u) => { asked.push(u); return /960px/.test(u) ? refused : ok(); } });
++    cache = createPhotoCache({ config, s: { db }, objectsClient, uploadGapMs: 0, log: { warn() {} }, fetchImpl: async (u) => { asked.push(u); return /960px/.test(u) ? refused : ok(); } });
      out = await cache.cache(image);
      assert.deepStrictEqual(asked.map((u) => u.replace(/.*\//, '')), ['960px-Kubota.jpg', '1280px-Kubota.jpg']);
      assert.ok(out && out.url.startsWith('https://openvibe.media/'), 'copied from the original URL');
  
      // 3. Both refused: null, and the log says why.
      const warned = [];
--    cache = createPhotoCache({ config, s: { db }, objectsClient, log: { warn: (m) => warned.push(m) }, fetchImpl: async () => refused });
++    cache = createPhotoCache({ config, s: { db }, objectsClient, uploadGapMs: 0, log: { warn: (m) => warned.push(m) }, fetchImpl: async () => refused });
      out = await cache.cache(image);
      assert.strictEqual(out, null);
      assert.ok(warned.some((m) => /answered 400/.test(m)), 'a refused photo is logged, not silent');
++    // 4. Uploads are paced: two copies are at least the gap apart.
++    const times = [];
++    const paced = createPhotoCache({ config, s: { db }, uploadGapMs: 150, log: { warn() {} }, fetchImpl: async () => ok(),
++        objectsClient: { upload: async () => { times.push(Date.now()); return { id: 'o', public_url: 'https://openvibe.media/api/v2/tours/objects/o' }; } } });
++    await Promise.all([paced.cache({ ...image, sourceUrl: image.sourceUrl + '1' }), paced.cache({ ...image, sourceUrl: image.sourceUrl + '2' })]);
++    assert.ok(times.length === 2 && times[1] - times[0] >= 140, 'one upload per gap');
      console.log('photo cache: 960px first, the imageinfo URL as fallback, refusals logged');
  })().catch((e) => { console.error(e); process.exitCode = 1; });