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.

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

[Echo] Try and fetch new expired slots on sending; Convert all images to JPEGs #4778

Merged
merged 2 commits into from
Jan 9, 2024

Conversation

dracos
Copy link
Member

@dracos dracos commented Jan 3, 2024

Hopefully resolving two reasons we get stuck/problem reports:

  1. Some backends have a maximum filesize for images. This has only ever been hit by PNGs people occasionally upload. Rather than limit this to those backends or PNGs, easier to convert all uploaded images to JPEGs.
  2. If a report to Echo fails due to expired slots for some reason (we do check at report creation but if there's another issue sending (e.g. issue 1 above) this could happen), try and fetch new ones immediately for next retry rather than await manual interaction.

@dracos dracos requested a review from davea January 3, 2024 16:12
Copy link

codecov bot commented Jan 3, 2024

Codecov Report

Attention: 18 lines in your changes are missing coverage. Please review.

Comparison is base (0eac24d) 85.34% compared to head (a92d737) 52.90%.

Files Patch % Lines
perllib/FixMyStreet/Roles/CobrandEcho.pm 55.55% 9 Missing and 7 partials ⚠️
perllib/FixMyStreet/Roles/CobrandSLWP.pm 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #4778       +/-   ##
===========================================
- Coverage   85.34%   52.90%   -32.44%     
===========================================
  Files         341      321       -20     
  Lines       24647    23608     -1039     
  Branches     4673     4451      -222     
===========================================
- Hits        21035    12490     -8545     
- Misses       2191     9861     +7670     
+ Partials     1421     1257      -164     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@davea davea left a comment

Choose a reason for hiding this comment

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

One typo, otherwise looks good! Is there a risk that if Echo somehow always returned Selected reservations expired for every attempt a report would be tried over and over again with little throttling? Guess it's unlikely, however.

perllib/FixMyStreet/Roles/CobrandEcho.pm Outdated Show resolved Hide resolved
@dracos dracos force-pushed the convert-all-to-jpeg branch from dc6fead to 6f13429 Compare January 4, 2024 15:48
@dracos dracos force-pushed the convert-all-to-jpeg branch from 6f13429 to a92d737 Compare January 9, 2024 09:31
@dracos dracos merged commit a92d737 into master Jan 9, 2024
19 of 21 checks passed
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