Closed Bug 1174253 Opened 9 years ago Closed 9 years ago

Refactor the addon <-> tag m2m with an implicit through model

Categories

(Marketplace Graveyard :: Code Quality, defect)

Avenir
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
2015-06-23

People

(Reporter: robhudson, Assigned: robhudson)

References

Details

The AddonTag model seems to only exist to change the table name. If this were an implicit model it would be easier to make Tags work with both Webapps and Websites. This should be a simple table rename and code refactor.
https://github.com/mozilla/zamboni/commit/23685e2 

To verify, please ensure any areas that can set/add tags/keywords have no regressions.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2015-06-23
Our marketplace.dev.developer_hub.saucelabs suite's run[1] hit quite a few of these errors:

http://sentry.dmz.phx1.mozilla.com/marketplace-dev/marketplace-dev/group/27895/

MySQLdb.connections in defaulterrorhandler

OperationalError: (1054, "Unknown column 'addons_tags.webapp_id' in 'where clause'")

Stacktrace (most recent call last):

  File "django/core/handlers/base.py", line 111, in get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "django/db/transaction.py", line 394, in inner
    return func(*args, **kwargs)
  File "django/db/transaction.py", line 394, in inner
    return func(*args, **kwargs)
  File "newrelic/packages/wrapt/wrappers.py", line 454, in __call__
    args, kwargs)
  File "newrelic/hooks/framework_django.py", line 492, in wrapper
    return wrapped(*args, **kwargs)
  File "mkt/site/decorators.py", line 117, in wrapper
    return f(*args, **kw)
  File "mkt/site/decorators.py", line 104, in __call__
    return self.f(*args, **kw)
  File "mkt/webapps/decorators.py", line 43, in wrapper
    return f(request, addon, *args, **kw)
  File "mkt/site/decorators.py", line 31, in wrapper
    return func(request, *args, **kw)
  File "mkt/developers/decorators.py", line 64, in wrapper
    return fun()
  File "mkt/developers/decorators.py", line 27, in fun
    return f(request, addon_id=addon.id, addon=addon, *args, **kw)
  File "mkt/developers/views.py", line 260, in status
    'is_tarako': addon.tags.filter(tag_text=QUEUE_TARAKO).exists(),
  File "django/db/models/query.py", line 606, in exists
    return self.query.has_results(using=self.db)
  File "django/db/models/sql/query.py", line 457, in has_results
    return compiler.has_results()
  File "django/db/models/sql/compiler.py", line 757, in has_results
    return bool(self.execute_sql(SINGLE))
  File "django/db/models/sql/compiler.py", line 786, in execute_sql
    cursor.execute(sql, params)
  File "django/db/backends/utils.py", line 65, in execute
    return self.cursor.execute(sql, params)
  File "django/db/utils.py", line 94, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "django/db/backends/utils.py", line 65, in execute
    return self.cursor.execute(sql, params)
  File "django/db/backends/mysql/base.py", line 129, in execute
    return self.cursor.execute(query, args)
  File "newrelic/hooks/database_dbapi2.py", line 22, in execute
    *args, **kwargs)
  File "MySQLdb/cursors.py", line 205, in execute
    self.errorhandler(self, exc, value)
  File "MySQLdb/connections.py", line 36, in defaulterrorhandler
    raise errorclass, errorvalue

[1] https://webqa-ci.mozilla.com/job/marketplace.dev.developer_hub.saucelabs/212/
Mat: Any ideas? I'm not sure where it's picking up the `webapp_id` part. It doesn't seem like it's part of my `_m2m_name` code? I'll look more tomorrow when I get online but if you have a few minutes to take a quick look I'd appreciate it.
Flags: needinfo?(mpillard)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I reproduced it locally. We overlooked something when renaming the table: the old model was using a field called 'addon' to point to Webapp, but the new, automatic model wants the field (and therefore the db column) to be called 'webapp'. (This is why tests are passing - they are using a fresh db)

There doesn't seem to be a way to customize the column name with automatic m2ms in django, so we'll need to make a migration renaming the column (and hoping that doesn't take too long...) to fix it :(
Flags: needinfo?(mpillard)
We encountered some 500 code errors on MP-dev FF41(Win 7):
1. Accessing Statistics page for any app https://bugzilla.mozilla.org/show_bug.cgi?id=1175486
2. Accessing Edit Listing and Status/Verions (Developer Hub - My Submissions) for any app (http://screencast.com/t/gYfvfYBmY9Xc)
3. Accessing Comm Dash for any app (http://screencast.com/t/PHKA89zKVv8L)
Other places where the 500 error occurs:
- reviewer tools when accesing an app review detail page: http://screencast.com/t/DNbzxELy
- opening an app in lookup tool: http://screencast.com/t/eH8MSfFJsR8
- when creating a featured app in curation tool (or a collection editorial brand etc.): http://screencast.com/t/rzsF26Dekk
Writing the migration to fix this is non trivial because in MySQL, you need to drop and re-create FK constraints to do that. And it looks like the FK name is inconsistent on our servers: everyone seems to be using `addons_tags_ibfk_3` (addons_tags_ibfk__2 points the tag, addons_tags_ibfk__1 does not exist) but dev has `addon_id_refs_id_38d90473` for some reason ?
https://github.com/mozilla/zamboni/commit/eca609d4
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
The scenarios from comment 5 and comment 6 are working now.
I have encountered the following issue that I think is a regression from this bug: http://screencast.com/t/XMGbFPaEq8wF
When setting search keyword in the submission process, the process can be completed and fails with an "Oops.." error.

Reopening bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://github.com/mozilla/zamboni/commit/6c11e84d54e38570aa060c587e8fd216bdd29e17
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Issue from comment 9 is no longer reproducing. Also adding, removing tags, search by tags works fine.
Postfix screencast: http://screencast.com/t/8ev5CZrZyIg
Closing bug.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.