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.

Sign up for GitHub

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

Jump to bottom

MBS-10469: Add 10 minute delay to shown CAA on front page #1540

Merged
merged 1 commit into from Jun 1, 2020
Merged

MBS-10469: Add 10 minute delay to shown CAA on front page #1540

merged 1 commit into from Jun 1, 2020

Conversation

Freso
Copy link
Member

@Freso Freso commented May 29, 2020 •
edited by yvanzo

Problem

Currently a lot of cover art on the front page is not shown since it hasn’t been processed by the IA yet, so visitors see "Image not available yet" messages instead.

Solution

MBS-10469: The proper solution to fix this would be to not show images on the front page until they’re actually processed, but I am not currently aware of a way to do this easily and reliably (there are database columns for storing sizes of the thumbnails, but they don’t seem to be populated, otherwise they would possibly be a good fit).

This is mostly meant as a temporary workaround to lessen the problem. It won’t remove the problem entirely, since sometimes cover arts take more than 10 minutes to generate, but having a 3+ hour long wait also seems excessive.

Note

I don’t have access to a live test server, so I’ve only played around with the query in my local database copy, not actually tried it out on a running copy of the server code. Also, this is mostly meant as a proof-of-concept. I am happy to have this PR thrown away for a better solution. This is just to hopefully inspire some thought on the matter. :)

@Freso
MBS-10469: Add 10 minute delay to shown CAA on front page
1ddc79d 
Currently a lot of cover art on the front page is not shown since it
hasn’t been processed by the IA yet, so visitors see "Image not
available yet" messages instead. The proper solution to fix this would
be to not show images on the front page until they’re actually
processed, but I am not currently aware of a way to do this easily and
reliably (there are database columns for storing sizes of the
thumbnails, but they don’t seem to be populated, otherwise they would
possibly be a good fit).

This is mostly meant as a temporary workaround to lessen the problem. It
won’t remove the problem entirely, since sometimes cover arts take more
than 10 minutes to generate, but having a 3+ hour long wait also seems
excessive.
@brainzbot
Copy link
Member

Can one of the admins verify this patch?

yvanzo
yvanzo approved these changes May 29, 2020
View reviewed changes
Copy link
Contributor

@yvanzo yvanzo left a comment

Choose a reason for hiding this comment

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

Thanks. I would not mind increasing the delay if that helps to significantly prevent this issue from occurring.

reosarevok
reosarevok approved these changes May 30, 2020
View reviewed changes
Copy link
Member

@reosarevok reosarevok left a comment

Choose a reason for hiding this comment

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

If you've tested the query and it works, this LGTM.

@yvanzo
Copy link
Contributor

yvanzo commented Jun 1, 2020

I tested it, see also https://www.postgresql.org/docs/12/functions-datetime.html.

But, when linking to the related ticket, I noticed that even though we cannot know if a thumbnail has been generated from a database query, it depends on ticket MBS-7480 which is assigned to @mwiencek, so it would be nice to have his review at least.

@yvanzo yvanzo requested a review from mwiencek June 1, 2020 08:31
mwiencek
mwiencek approved these changes Jun 1, 2020
View reviewed changes
Copy link
Member

@mwiencek mwiencek left a comment

Choose a reason for hiding this comment

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

Makes sense to me

@yvanzo yvanzo merged commit 6add14f into metabrainz:master Jun 1, 2020
@Freso Freso deleted the MBS-10469-delay-showing-CAA-on-front-page branch June 1, 2020 16:29
yvanzo added a commit that referenced this pull request Jun 10, 2020
@yvanzo
Merge branch 'master' into beta
97841a5 
* master:
  Update POT files using the production database
  MBS-1736: Block setting format on too early releases (#1111)
  MBS-10697 / MBS-10725 / MBS-10758 / MBS-10848: Smart links (#1544)
  MBS-6864: Drop all PUID-related code (#1551)
  Eslint fixes + typographic apostrophe
  MBS-4644: Show releases that have CAA images in lists
  MBS-10873: Load ACs for annotation edits (#1541)
  MBS-6532: Show work edits in recording / release history (#1165)
  MBS-10381: Disallow using a work language multiple times (#1559)
  MBS-10892: Show private collaborated-on collections on sidebars (#1565)
  MBS-8625: release data quality icons (#1556)
  Fix invalid <div> inside <span>
  Compile static resources in the docker images
  Remove DBDefs dependency from webpack config
  Make DBDefs JS external from build
  Add DBDefs Flow defs
  Cleanup dbdefs_to_js conversion
  Remove unused variable
  MBS-10893: Document selecting artist for CD lookup (#1563)
  MBS-10890: Do not load language for works multiple times (#1562)
  Fix error message about Instagram `/explore` URLs (#1566)
  MBS-10821: Remove orphaned recordings from collections for deletion (#1512)
  MBS-10862: Report "CatNoLooksLikeLabelCode" (#1548)
  MBS-9931: Fail gracefully when trying to remove example rel
  MBS-9340: Only allow mul and zxx as the only work language
  Set cacheDirectory: true for babel-loader
  Set preset-env target to node version on server
  Bump babel-loader to 8.1.0
  Fix multiple imports on one line
  Upgrade core-js to v3 and set useBuiltIns: 'usage'
  Fix format_editnote running on localized note URL
  MBS-10186: Check for missing CD TOC on SetTrackLengths (#1511)
  Upgrade all babel deps
  Bump mutate-cow to 4.0.2
  MBS-10891: Replace Magic MP3 Tagger link with AudioRanger (#1560)
  Escape ', ", `, and GIT_MSG in GIT_MSG (#1557)
  MBS-10885: Fix error when URL has been removed (#1553)
  Eslint fixes
  MBS-10887: Fix country/date text CSS alignment (#1554)
  Refactor 435310d: Check DBDefs instead of stash (#1555)
  Fix TypeError when changing relationship type
  MBS-10856: Trim whitespace on relationship credits (#1545)
  Fix non-sanitized staging server is beta not test
  Disable IP hashes on sanitized staging server
  MBS-10836: Fix edit / remove collection links (#1550)
  Drop FUNDING file made obsolete by '.github' repo
  MBS-10882: Remove accidental duplicate push (#1547)
  Rename AddStandaloneRecordingProps and add separate props type
  $EDIT_RECORDING_CREATE artist_credit is not optional
  Fix bracket indentation
  $EDIT_RECORDING_CREATE name is not optional
  MBS-10771: Block tagging for unverified users (#1504)
  MBS-10877: Display source URLs in the relationship editor dialog
  MBS-10867: Fix "entity not in the entities array"
  MBS-10469: Add 10 minute delay to shown CAA on front page (#1540)
  Only print recording name instead of linking recording twice
  MBS-10393: Convert Add Standalone Recording edit to React
  Add react-dom libdefs from flow-typed
  Document $c-in-JS a bit more
  Enable types-first mode in Flow
  Add Flow types to AreaWithContainmentLink.js
  Add Flow types to AreaContainmentLink.js
  Add Flow types to 404.js
  Add Flow types to Footer.js
  Pass $c to all server-rendered components
  Add Flow types to ExternalLinks.js
  Bump Flow to 0.125.1
  Fix typo preventing to store alert_mtime on beta
  Add colon instead of brackets to row header
yvanzo added a commit that referenced this pull request Jun 15, 2020
@yvanzo
Merge branch 'beta' into production
c3c6348 
* beta:
  Update translations from Transifex
  Fix warning about masked variable since c1faa8b
  Update POT files using the production database
  MBS-1736: Block setting format on too early releases (#1111)
  MBS-10697 / MBS-10725 / MBS-10758 / MBS-10848: Smart links (#1544)
  MBS-6864: Drop all PUID-related code (#1551)
  Eslint fixes + typographic apostrophe
  MBS-4644: Show releases that have CAA images in lists
  MBS-10873: Load ACs for annotation edits (#1541)
  MBS-6532: Show work edits in recording / release history (#1165)
  MBS-10381: Disallow using a work language multiple times (#1559)
  MBS-10892: Show private collaborated-on collections on sidebars (#1565)
  MBS-8625: release data quality icons (#1556)
  Fix invalid <div> inside <span>
  Compile static resources in the docker images
  Remove DBDefs dependency from webpack config
  Make DBDefs JS external from build
  Add DBDefs Flow defs
  Cleanup dbdefs_to_js conversion
  Remove unused variable
  MBS-10893: Document selecting artist for CD lookup (#1563)
  MBS-10890: Do not load language for works multiple times (#1562)
  Fix error message about Instagram `/explore` URLs (#1566)
  MBS-10821: Remove orphaned recordings from collections for deletion (#1512)
  MBS-10862: Report "CatNoLooksLikeLabelCode" (#1548)
  MBS-9931: Fail gracefully when trying to remove example rel
  MBS-9340: Only allow mul and zxx as the only work language
  Set cacheDirectory: true for babel-loader
  Set preset-env target to node version on server
  Bump babel-loader to 8.1.0
  Fix multiple imports on one line
  Upgrade core-js to v3 and set useBuiltIns: 'usage'
  Fix format_editnote running on localized note URL
  MBS-10186: Check for missing CD TOC on SetTrackLengths (#1511)
  Upgrade all babel deps
  Bump mutate-cow to 4.0.2
  MBS-10891: Replace Magic MP3 Tagger link with AudioRanger (#1560)
  Escape ', ", `, and GIT_MSG in GIT_MSG (#1557)
  MBS-10885: Fix error when URL has been removed (#1553)
  Eslint fixes
  MBS-10887: Fix country/date text CSS alignment (#1554)
  Refactor 435310d: Check DBDefs instead of stash (#1555)
  Fix TypeError when changing relationship type
  MBS-10856: Trim whitespace on relationship credits (#1545)
  Fix non-sanitized staging server is beta not test
  Disable IP hashes on sanitized staging server
  MBS-10836: Fix edit / remove collection links (#1550)
  Drop FUNDING file made obsolete by '.github' repo
  MBS-10882: Remove accidental duplicate push (#1547)
  Rename AddStandaloneRecordingProps and add separate props type
  $EDIT_RECORDING_CREATE artist_credit is not optional
  Fix bracket indentation
  $EDIT_RECORDING_CREATE name is not optional
  MBS-10771: Block tagging for unverified users (#1504)
  MBS-10877: Display source URLs in the relationship editor dialog
  MBS-10867: Fix "entity not in the entities array"
  MBS-10469: Add 10 minute delay to shown CAA on front page (#1540)
  Only print recording name instead of linking recording twice
  MBS-10393: Convert Add Standalone Recording edit to React
  Add react-dom libdefs from flow-typed
  Document $c-in-JS a bit more
  Enable types-first mode in Flow
  Add Flow types to AreaWithContainmentLink.js
  Add Flow types to AreaContainmentLink.js
  Add Flow types to 404.js
  Add Flow types to Footer.js
  Pass $c to all server-rendered components
  Add Flow types to ExternalLinks.js
  Bump Flow to 0.125.1
  Fix typo preventing to store alert_mtime on beta
  Add colon instead of brackets to row header
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mwiencek mwiencek mwiencek approved these changes

@reosarevok reosarevok reosarevok approved these changes

@yvanzo yvanzo yvanzo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@Freso @brainzbot @yvanzo @mwiencek @reosarevok