models mandatory video_data field

[stereoit]

trafficstars

I had an issue with user just pasting url, e.g. https://www.youtube.com/watch?v=gDLKwTdurxg and then clicking "save" without anything else. It basically does not process the video and after save the plugin fails on:

Exception Value:    
type object argument after ** must be a mapping, not NoneType
Exception Location: /usr/lib/python2.7/site-packages/djangocms_youtube/models.py in video, line 76

Which really users vide_data as kwargs:

    @property
    def video(self):
        cls = Video(**self.video_data)
        return cls

It is enough for me to mark this field as mandatory. Probably updated migration is needed, but I have not idea how to do it here on GH.

[mishbahr]

Thanks for your pull request.

Ans as you have mentioned, model changes requires migrations for for both djnago and south.

I can't merge the the changes as it is, sorry! Please add migrations or I'll try to do it in the master branch as soon as I can.

Regards

Mishbah

[stereoit]

Can you give me hint how to actually do it? Do I need to copy the project to my django-app and run migrations from there and then copy back? And how to keep south migration and django migrations in sync?

Robert

On Wed, Jan 20, 2016 at 4:25 PM, Mishbah Razzaque [email protected] wrote:

Thanks for your pull request.

Ans as you have mentioned, model changes requires migrations for for both djnago and south.

I can't merge the the changes as it is, sorry! Please add migrations or I'll try to do it in the master branch as soon as I can.

Regards

Mishbah

— Reply to this email directly or view it on GitHub https://github.com/mishbahr/djangocms-youtube/pull/1#issuecomment-173237186 .

stereoIT s.r.o. Heřmanova 23 107 00 Praha 7 mob: +420 776 76 23 78

[mishbahr]

You'll have to setup 2 projects (django 1.6.x with South and django >=1.7), fork this repo and pip install locally (see https://pip.pypa.io/en/stable/reference/pip_install/#editable-installs)

[mishbahr]

Alternatively, please wait few days and I'll try to fix is as soon as I can.

[stereoit]

Hi,

I've tried to setup local development, but installing the editable version, brings django.1.9.2 and when I downgrade, the sample app starts to complain quite a lot about missing dependencies. Seems I won't be able to push those changes, can you kidnly do it?

Regards,

Robert

On Wed, Jan 20, 2016 at 4:59 PM, Mishbah Razzaque [email protected] wrote:

Alternatively, please wait few days and I'll try to fix is as soon as I can.

— Reply to this email directly or view it on GitHub https://github.com/mishbahr/djangocms-youtube/pull/1#issuecomment-173248844 .

stereoIT s.r.o. Heřmanova 23 107 00 Praha 7 mob: +420 776 76 23 78

[leesolway]

Caught me out, breaking projects.

[mishbahr]

Ideally I would like to keep video_data field optional .. change internal logic to return empty strings or sensible data e.g duration = 0 sec when video data is not available.

Unfortunately I'm super busy ... to make the suggested amends myself.. so pull requests are welcome!

[aidan-doherty]

Just got the same error can the pull request be merged to master yet ?

[mishbahr]

This merge request is not complete .. missing migrations for both django and south.

[jkaikko]

Just a heads up to anyone experiencing this. I think the core problem might be that the google api responds really slowly and by the time people click on the save button, the video data has not been fetched (and video_data field is empty). In my case, my server was also timing out before the api request finished. Increasing nginx timeout and just waiting for the request helped.

But, who has time to wait 2 minutes for the api request to finish everytime you add a youtube video...

[aidan-doherty]

@jkaikko Its not getting as slow as that for me but making the video data field unique prevents the save and stops it from breaking the page you are adding it to. I have just created my own local version of the app without south migrations and made the required fix until it is updated on this git repo.