Opened 4 years ago

Last modified 2 years ago

#5437 in_progress defect

blog media_type uses inconsistent naming

Reported by: ayleph Owned by: ayleph
Priority: major Milestone:
Component: programming Keywords: blog, models, error
Cc: Parent Tickets: #5438

Description

The blog media_type hasn't been working for quite some time due to naming conflicts. The problem was introduced in 511ce0dwhen mediagoblin/media_types/blog/__init__.py was changed as below.

--- a/mediagoblin/media_types/blog/__init__.py
+++ b/mediagoblin/media_types/blog/__init__.py
@@ -18,7 +18,7 @@ from mediagoblin.media_types import MediaManagerBase

 from mediagoblin.tools import pluginapi

-MEDIA_TYPE = 'mediagoblin.media_types.blog'
+MEDIA_TYPE = 'mediagoblin.media_types.blogpost'

This causes new blog entries to be added to core__media_entries with media_type = mediagoblin.media_types.blogpost. However, all the blog code is located at mediagoblin/media_types/blog. This causes server errors like below.

...
File '/path/to/mediagoblin/mediagoblin/db/models.py', line 665 in media_data
  return getattr(self, self.media_data_ref)
File 'build/bdist.linux-x86_64/egg/sqlalchemy/util/langhelpers.py', line 754 in __get__
File '/path/to/mediagoblin/mediagoblin/db/models.py', line 687 in media_data_ref
  return import_component(self.media_type + '.models:BACKREF_NAME')
File '/path/to/mediagoblin/mediagoblin/tools/common.py', line 34 in import_component
  __import__(module_name)
ImportError: No module named blogpost.models

I fixed this long ago on one of my instances. I'll post the patch and migration that would be required to fix it on someone else's instance.

Subtickets

Attachments (2)

0001-Partial-fix-5437-Blog-mediatype-naming.patch (1.6 KB) - added by ayleph 4 years ago.
0002-Migration-5437-Blog-mediatype-naming.patch (2.5 KB) - added by ayleph 4 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 4 years ago by ayleph

Status: newreview

Attached patch and migration correct the media_type name and update and blogpost entries already in the database.

comment:2 Changed 4 years ago by ayleph

Also sent an email to Aditi (original author) and Sebastian Späth to inquire about the commit mentioned in the description.

comment:3 Changed 4 years ago by spaetz

Hi, I do not remember the specifics. But afaik this was because each entry is a blogpost and not a blog, just as each image or video is not a gallery.

If the change back does not seem to cause problems, I can't think of a reason not to applay your patch.

comment:4 in reply to:  3 Changed 4 years ago by ayleph

Replying to spaetz:

Hi, I do not remember the specifics. But afaik this was because each entry is a blogpost and not a blog, just as each image or video is not a gallery.

Thanks for your input! I can understand the rationale behind the name change, but it looks like the change was incomplete. Changing the model name without changing all of the underlying functions and the path to the plugin seems to have broken the plugin. I was wondering if we were missing a patch set somewhere that applied the rest of the accompanying changes.

If the change back does not seem to cause problems, I can't think of a reason not to applay your patch.

The patch and migration I proposed to change the model name back to the original "blog" instead of "blogpost" tests out fine for me, but I'd like to have someone else review to confirm. It seems like the easiest way back to a functioning plugin. If your or Aditi have some other code or solutions, I'm interested to see them.

comment:5 Changed 4 years ago by ayleph

Owner: set to ayleph
Status: reviewin_progress

From discussions in IRC, it came to my attention that the migration should be part of the plugin migrations, not part of the main mediagoblin migrations. I'll need to do some work to address that.

comment:6 Changed 4 years ago by ayleph

Parent Tickets: 5438

comment:7 Changed 2 years ago by ayleph

I made a migration for this, but now I'm not sure it's even an issue anymore. I spun up a brand new instance recently from master, and the blog plugin worked out of the box without making any changes to the mediatype name. I need to investigate a little more before I'm ready to close this as invalid though.

Note: See TracTickets for help on using tickets.