Lives and shorts

[simpleman19]

No description provided.

[bbilly1]

Excellent work on this one! At first glance this all makes a lot of sense, I see, I can retire now. :-)

Good call on making the channel page sizes configurable for the different types, very nice! I think at some point, we'll have to implement that as a per channel basis as well, but we can still look into that later.

In the mapping, at first glance I was thinking that we can combine vid_type and was_live? As the vid_type can by any of the VideoTypeEnum including live? Or am I missing something?

I was thinking we should have a startup check that sets the vid_type to video for all current videos, this would simplify the query in the view. Also some videos are deactivated, then they will never get that field set. I remember creating a mess with a similar situation before, where an expected key wasn't set, a few versions later, so it was hard to debug.

Then the refresh task will categorize the videos correctly, or as now implemented, there is a per channel refresh button to trigger that manually.

Are you up to continue with that? Or shall I add to your PR? Up to you. Also there are some linting issues... :-)

[simpleman19]

I can remove the was_live flag, it's probably not necessary since vid_type will be set with "live" if it was a live video.

I will look at adding a startup check to set all vid_type to video if not set, I am honestly not super familiar with Django or ElasticSearch but I agree that would be much cleaner when querying back out.

[bbilly1]

For the startup check, there are a bunch of methods defined already in the StartupCheck class.

You can add another method there. That could look something like this:

def es_set_vid_type(self):
    """update path 0.3.0 to 0.3.1, set default vid_type to video"""
    data = {
        "query": {
            "bool": {"must_not": [{"exists": {"field": "vid_type"}}]}
        },
        "script": {"source": "ctx._source['vid_type'] = 'video'"},
    }
    response, _ = ElasticWrap("ta_video/_update_by_query").post(data=data)
    print(f"index update ran: {response}")

_update_by_query [docs] is a good choice for bulk updates like that, the query must_not exist on the vid_type field [docs] makes sure that it wont overwrite things in the future.

A classic gotcha would be, you need to call it after the ElasitIndexWrap().setup() which is responsible to set new mappings.

I hope this helps!

[simpleman19]

@bbilly1 Pushed updates to add startup checks, remove was_live flag, and applied the black linter to the files changed

[simpleman19]

Apologies, didn't realize that only one of them ran at a time and bailed afterwards if they failed so I only fixed the stuff out of black initially. I actually ran the deploy validate now and made the corresponding updates"

[bbilly1]

There you go, linter is passing!

Unfortunately I have to manually approve the action run for first time contributors, you know, you could be running cryptominers here. :-) but that's going to be easier in the future.

Thanks a lot for your help on this, I'll take look at it next year. :-)

[simpleman19]

Awesome, I think I finally got my local linter setup correctly, I guess I was missing one of the plugins for flake8 so I should finally have my env setup :)

Have a happy new year!

[bbilly1]

Excellent! I think that gets us 95% of the way there. I'll do some fine tuning, but that was a big help!