1202409 - Remove thumbtype field from Preview model

Status: RESOLVED FIXED

(Marketplace Graveyard :: Code Quality, defect, P5)

Description

[Mathieu Pillard [:mat]]

The Preview model has a thumbtype field, but that field is not used anywhere in our code. We don't even set it when a developer uploads a new screenshot, only fakedata sets it.

Let's remove that field. A couple tests in mkt/webapps/tests/ and a few lines of code in mkt/webapps/fakedata.py will need to be changed once the field is removed from the model.

Comment 1

[Srikar Reddy Demagu [:srikar]]

I am a new to marketplace. Can I work on this ?

Comment 2

[Mathieu Pillard [:mat]]

Assuming you are familiar with python, install Marketplace backend http://marketplace.readthedocs.org/en/latest/topics/backend.html. Familiarise yourself with the code and the backend documenation, then:
- Remove the field itself from mkt/webapps/models.py, in the Preview class
- Create a migration to reflect that change. If everything is ok you should just need to run `manage.py makemigrations` from the zamboni container shell to generate it
- Remove all code in zamboni that deals with the "thumbtype" field on the "Preview" model (as mentioned in comment 0, there are a few tests and some code in mkt/webapps/fakedata.py that needs to be changed)
- Submit a pull request with your changes!

Comment 3

[Mathieu Pillard [:mat]]

I'm assigning the bug to you. Let me know if you have more questions.

Comment 4

[Srikar Reddy Demagu [:srikar]]

Yeah, I am familiar with Python. I will start by installing the marketplace back end and diving into the code. Thank you

Comment 5

[Srikar Reddy Demagu [:srikar]]

Do I need to run the tests after making changes and before submitting a pull request ?

Comment 6

[Mathieu Pillard [:mat]]

It'd be best, but if the tests are too slow for you locally, just submit the pull request, tests are ran automatically by https://travis-ci.org/ on every pull request.

Comment 7

[Srikar Reddy Demagu [:srikar]]

Hi Mathieu

Though I am finished up making modifications in the code, I am having hard time while running migrations using ./manage.py migrate. Below is the output I am getting when I run the command. 

(zamboni)srikar@OnFire:~/mozilla/zamboni$ ./manage.py migrate
Traceback (most recent call last):
  File "./manage.py", line 45, in <module>
    from lib.utils import update_csp, validate_modules, validate_settings  # noqa
  File "/home/srikar/mozilla/zamboni/lib/utils.py", line 1, in <module>
    import jwt
  File "/home/srikar/.virtualenvs/zamboni/local/lib/python2.7/site-packages/jwt/__init__.py", line 10, in <module>
    import M2Crypto
  File "/home/srikar/.virtualenvs/zamboni/local/lib/python2.7/site-packages/M2Crypto/__init__.py", line 22, in <module> 
    import __m2crypto
ImportError: libssl.so.10: cannot open shared object file: No such file or directory


Can you give any pointers on how to resolve this?

Best Regards
Srikar

Comment 8

[Mathieu Pillard [:mat]]

Were you following the docker instructions ? 

To fix it, from your virtualenv, try installing M2Crypto package from pypi instead of our custom pip repos:
$ pip install M2Crypto==0.22.3 -I --no-deps

Comment 9

[David Durst [:ddurst]]

Any progress on this? If this is still unchanged in two weeks, we'll unassign it.

Comment 10

[Wasim Thabraze]

Hello Mathieu,

I'm new to Bugzilla and I would like to work on this bug.

I have a good command over Python and have already contributed to few projects on GitHub.

Can you please assign this bug to me? I'm really interested in fixing this bug.

Regards,

Wasim
https://github.com/waseem18

Comment 11

[Mathieu Pillard [:mat]]

Sure. See comment 2 for details.

Comment 12

[Wasim Thabraze]

Thank you Mathieu. I have started studying the code and will update the progress soon!

Comment 13

[Wasim Thabraze]

Hello Mathieu,

Here are the changes I made. https://github.com/mozilla/zamboni/pull/3471

Can you please review the PR?
 

Thank you!

Comment 14

[Wasim Thabraze]

Hello Mathieu,

I'm trying to create migrations but I'm getting this error.(http://i.imgur.com/IAOxG6g.png)

I have installed all the requirements specified. But still I'm getting this error. Can you please help me out in resolving it?

Comment 15

[Mathieu Pillard [:mat]]

Are you using docker or not ? If you are using docker, then it sounds like you are not running the command in a docker shell as you should - prefix the command with "docker-compose run zamboni" without quotes or start a zamboni shell with "docker-compose run zamboni bash" and then run your command normally inside that shell.

Comment 16

[Wasim Thabraze]

Hello Mathieu,


As you have said, I ran the command 'docker-compose run zamboni python manage.py createmigrations'.

Running this command shows this error (http://i.imgur.com/3UfrGgn.png)[Error at the bottom]. I have also installed 'rest_framework'. But I still get that error.

I tried asking for help on IRC but it's been around 12 hours, I didn't get any reply. Could you please help me out in this issue?

Thanks

Comment 17

[Mathieu Pillard [:mat]]

You weren't on IRC when I was :(

This error sounds like the code in your environment is not up to date with the dependencies it needs to run. Try this: http://marketplace.readthedocs.org/en/latest/topics/backend-details.html#how-do-i-update-python-node-package-deps-rebuild-the-container

Comment 18

[Wasim Thabraze]

Hello Mathieu,

I finally resolved that issue and was able to create migrations. After removing thumbtype's from the files, I ran this command 'docker-compose run zamboni python manage.py makemigrations'.

It ran successfully producing a file in the Migrations folder.

Here are the contents of the file http://pastebin.com/5mePMc8L

Can you please review it and notify me if something is wrong or fine? I will send a PR if everything is fine.

Thank you. 
Happy New Yeah :)

Comment 19

[Mathieu Pillard [:mat]]

That sounds good, yeah. Happy New Year!

Comment 20

[Wasim Thabraze]

Hello Mathieu,

Here's the PR - https://github.com/mozilla/zamboni/pull/3478

Created migrations and added it.

Can you please review if the PR is fine and it resolves the bug?


Thank you.

Comment 21

[Mathieu Pillard [:mat]]

Fixed in https://github.com/mozilla/zamboni/commit/002045d38961e8407dbf9f9868849b893ca3d287

Thanks, Wasim!