search

the-blue-alliance / the-blue-alliance-android

An Android app for accessing information about the FIRST Robotics Competition.
MIT License
76 stars 34 forks source link

Consume media urls from api #885

Open phil-lopreiato opened 5 years ago

phil-lopreiato commented 5 years ago

Consume data exposed in https://github.com/the-blue-alliance/the-blue-alliance/pull/2368, so we don't have to be constructing image urls locally

bherbst commented 5 years ago

I ran into two issues when tackling this, both with the data the API exposed (PRs linked above).

YouTube image URLs are coming through with HTTP instead of HTTPS, so Pie devices refuse to load them (unless we whitelist that cleartext traffic, but there is no compelling reason to do so).

Picasso also doesn't like loading the CD media URLs because the web.archive.org links include an iframe (it isn't just a raw image).

phil-lopreiato commented 5 years ago

Huh, you're right on both fronts. Thanks for the other associated fixes - I merged them both :)

bherbst commented 5 years ago

@phil-lopreiato match videos also generate their URLs based on a key/type combo, but it doesn't look like the match endpoints expose view_url and direct_url.

Are you expecting that those endpoints should also expose the URLs, or do we want to just keep generating the match video URLs ourselves?

phil-lopreiato commented 5 years ago

Nice catch again. I guessed they would have, but maybe we need another tweak on the server side to add them (unsure why this didn't come along for free the first time)

phil-lopreiato commented 5 years ago

Oh, wait figured it out. This is a fun TBA quirk...

Match videos aren't technically stored as medias on the server side (they predate all other types of media). Instead, they're stored inline on the match object so they have their own format.

https://github.com/the-blue-alliance/the-blue-alliance/blob/master/models/match.py#L285

https://github.com/the-blue-alliance/the-blue-alliance/blob/master/database/dict_converters/match_converter.py#L37

So we should probably change this server-side as well.