Metro-Records / la-metro-councilmatic

:metro: An instance of councilmatic for LA Metro
MIT License
6 stars 2 forks source link

Download Agenda and Attachments Button Missing #1125

Open neilarellano opened 3 weeks ago

neilarellano commented 3 weeks ago

We noticed that Planning and Programming and Finance, Budget, and Audit Committee Meetings for this month are missing the “Download Agenda and Attachments” buttons.

The script for this second link may not be working.

image

xmedr commented 2 weeks ago

@antidipyramid The issue here is that while the event packets exist within the s3 bucket and staging/production databases have the correct slugs to find each event's packet, the full urls always point to the testing bucket. There is an event.packet.is_ready method in the event template that's in charge of displaying the download link, which returns true if the attachment url resolves with a 200 status code. Most attachments on both environments seem to exist in the testing bucket, and most resolve correctly. But there are a handful including the above Planning and Program Committee attachment that actually exist in the prod bucket instead.

Another example is this PTSC meeting. These can be tested using something like https://<bucket name>-testing.s3.amazonaws.com/<event slug>.pdf and removing the "-testing" portion of the url checks if it's in the prod bucket.

Do you have an idea what is uploading these attachments? I'm imagining a couple solutions here, and both could be used simultaneously:

  1. Adjust is_ready() to check the prod bucket if the url doesn't resolve.
  2. Figure out what part of the attachment uploader is only occasionally uploading to the prod bucket, and adjust it so everything from prod starts going to prod as intended. Then changing production to save packet urls as the prod bucket urls.
antidipyramid commented 1 week ago

@xmedr We set the attachment bucket URL as an environment variable in Heroku. It looks like the MERGE_HOST variable on the prod site was set to the testing URL so I imagine that's the source of the problem 🙂

We'll also need to update that variable in the prod Dashboard as well. After, we may need to do some manual work to update packet URLs in the database.

I'm actually not super familiar with the attachment logic so pairing on this might be a good way for both of us to learn more. What do you think?

xmedr commented 1 week ago

@antidipyramid That sounds great! I'll put a time on the calendar for next week, feel free to move it if you need to

xmedr commented 1 week ago

@hancush We figured out what was pushing packets over to testing as opposed to the production bucket, and have a fix coming to make sure that future packets/attachments always get uploaded to prod. The next task would be to correct previous urls in the councilmatic db to point to the right bucket. We have two options here and wanted to get your opinion on them:

  1. Write a script to identify the packets that councilmatic production is expecting that aren’t found in the testing bucket, and check for them in the production bucket. If found, it would adjust their urls in the db to point to the prod bucket instead. I imagine this would be done in a data migration. This would be the simpler of the two options and is my personal pick, since previous packets being stored in testing hasn’t been an issue.
  2. Identify which packets in the testing bucket actually apply to events in production, move them all over to the production bucket, and change every url to point to production. Considering there are 30k+ items in testing, this might take a while. This would probably need a data migration, as well as a script to manipulate all the items in those buckets. However, I could see the case for this, since it would make sure everything is stored in the same, correct place.
hancush commented 1 week ago

@xmedr I'd go programmatic. To confirm, you're proposing something like this?

# for packet in packets with testing urls
#      check for packet in prod bucket
#      if packet in prod bucket
#          update to prod url

Might I suggest an else clause that runs the merge if the packet is not in the prod bucket?

#     else
#         mark packet as not ready
#         run merge for packet

Conveniently, you can simply call the packet's save() method to run the merge, then it will update the URL for you.

https://github.com/Metro-Records/la-metro-councilmatic/blob/04df15e524a76cde55913f6b8d9c8de95851b5b9/lametro/models.py#L1087-L1096

hancush commented 1 week ago

N.b., this script / migration should run after the merge host is updated to the production bucket, if it hasn't been already.

xmedr commented 1 week ago

To confirm, you're proposing something like this?

Right! That's exactly what I was thinking. And thanks for those extra tips, I'll be using them.

this script / migration should run after the merge host is updated to the production bucket

Agreed. Councilmatic prod has already had its merge host updated, so I should be alright to get started on this.

antidipyramid commented 1 week ago

@antidipyramid will open a separate PR to update the dashboard environment variables with the correct endpoints as well.