Codecov Report

Merging #89 (aeb813a) into main (c98f609) will decrease coverage by 0.16%.
The diff coverage is 63.63%.

@@            Coverage Diff             @@
##             main      #89      +/-   ##
==========================================
- Coverage   95.88%   95.72%   -0.17%     
==========================================
  Files          48       48              
  Lines        2238     2247       +9     
==========================================
+ Hits         2146     2151       +5     
- Misses         92       96       +4     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c98f609...aeb813a. Read the comment docs.

OH! And when attaching the URI back to the session model, add the ?alt=media to the end. Unless that breaks the pipeline?

OH! And when attaching the URI back to the session model, add the ?alt=media to the end. Unless that breaks the pipeline?

Unfortunately this does break the pipeline when we try to do resource_copy on the file in GCS, we get a FileNotFoundError. Just curious, but what exactly was the reason for appending that to the URI?

OH! And when attaching the URI back to the session model, add the ?alt=media to the end. Unless that breaks the pipeline?

Unfortunately this does break the pipeline when we try to do resource_copy on the file in GCS, we get a FileNotFoundError. Just curious, but what exactly was the reason for appending that to the URI?

Oh whoops. I event added it in the wrong place myself. It is needed for google file storage to recognize that you aren't trying to download or copy the file but rather stream the file.

But what I meant by "I added it in the wrong place" is that you can keep it how it is for all the file storage and processing and such (exactly how it is now) but then during the database upload task, can you pass a parameter that says like "from_local" or something, and if it is local the session_video_uris need to be converted to their https:// equivalents. https://storage.googleapis.com.... Which is fortunately really easy to get:

In [1]: from gcsfs import GCSFileSystem

In [2]: fs = GCSFileSystem(token="../cdp-backend/.keys/cdp-jackson-dev-002-sa-dev.json")

In [3]: fs.url("cdp-jackson-dev-002.appspot.com/9af4adb4f28b34c0142a2ab28fa5603dd5fdb347c074d957d13636dae554ed34-audio.wav")
Out[3]: 'https://storage.googleapis.com/download/storage/v1/b/cdp-jackson-dev-002.appspot.com/o/9af4adb4f28b34c0142a2ab28fa5603dd5fdb347c074d957d13636dae554ed34-audio.wav?alt=media'

It even adds that ?alt=media to the URL!

TL;DR: it's need for the front-end streaming of the file and not the backend processing. So only change it right before database storage of the URI.

OH! And when attaching the URI back to the session model, add the ?alt=media to the end. Unless that breaks the pipeline?

Unfortunately this does break the pipeline when we try to do resource_copy on the file in GCS, we get a FileNotFoundError. Just curious, but what exactly was the reason for appending that to the URI?

Oh whoops. I event added it in the wrong place myself. It is needed for google file storage to recognize that you aren't trying to download or copy the file but rather stream the file.

But what I meant by "I added it in the wrong place" is that you can keep it how it is for all the file storage and processing and such (exactly how it is now) but then during the database upload task, can you pass a parameter that says like "from_local" or something, and if it is local the session_video_uris need to be converted to their https:// equivalents. https://storage.googleapis.com.... Which is fortunately really easy to get:

In [1]: from gcsfs import GCSFileSystem

In [2]: fs = GCSFileSystem(token="../cdp-backend/.keys/cdp-jackson-dev-002-sa-dev.json")

In [3]: fs.url("cdp-jackson-dev-002.appspot.com/9af4adb4f28b34c0142a2ab28fa5603dd5fdb347c074d957d13636dae554ed34-audio.wav")
Out[3]: 'https://storage.googleapis.com/download/storage/v1/b/cdp-jackson-dev-002.appspot.com/o/9af4adb4f28b34c0142a2ab28fa5603dd5fdb347c074d957d13636dae554ed34-audio.wav?alt=media'

It even adds that ?alt=media to the URL!

TL;DR: it's need for the front-end streaming of the file and not the backend processing. So only change it right before database storage of the URI.

Ah gotcha that makes sense, I didn't know it had that use case for the front end. Will do

Just curious but does the front end have a way of streaming the video of something stored in GCS using the HTTP url?

Not sure why the graphviz fails to install on ubuntu. I added some changes to the github workflow, but I think the workflow on this PR runs off of the main cdp_backend and not my changes.

Just curious but does the front end have a way of streaming the video of something stored in GCS using the HTTP url?

Yep! See:

From: https://councildataproject.org/seattle/#/events/2ee8dfd2-a4a5-416f-aded-f48b37e6e521

Not sure why the graphviz fails to install on ubuntu. I added some changes to the github workflow, but I think the workflow on this PR runs off of the main cdp_backend and not my changes.

They may have released something and something somewhere is broken in the dep chain, I will wait a bit and retrigger GitHub Actions to run.

This actually looks like a github actions release failure.... GitHub released a new build of their ubuntu runner 6 days ago which aligns with when the ubuntu tests started failing.... https://github.com/actions/virtual-environments/releases/tag/ubuntu20%2F20210816.1

It's actually not graphviz its ffmpeg it looks like.... The docs test works just fine. Investigating.

Got it figured out. Going to merge this then merge my PR to fix the builds without merge conflicts.