collective / collective.nitf

A Dexterity-based content type inspired on the News Industry Text Format specification
8 stars 3 forks source link

ValueError installing package #127

Closed hvelarde closed 9 years ago

hvelarde commented 9 years ago

If collective.cover is not installed, we get the following error when installing the package:

2015-05-28 15:32:23 ERROR Zope.SiteErrorLog 1432837943.470.80217005812 http://localhost:8080/@@plone-addsite
Traceback (innermost last):
  Module ZPublisher.Publish, line 138, in publish
  Module ZPublisher.mapply, line 77, in mapply
  Module ZPublisher.Publish, line 48, in call_object
  Module Products.CMFPlone.browser.admin, line 208, in __call__
  Module Products.CMFPlone.factory, line 105, in addPloneSite
  Module Products.GenericSetup.tool, line 365, in runAllImportStepsFromProfile
   - __traceback_info__: profile-collective.nitf:default
  Module Products.GenericSetup.tool, line 1119, in _runImportStepsFromContext
  Module Products.GenericSetup.tool, line 1030, in _doRunImportStep
   - __traceback_info__: plone.app.registry
  Module plone.app.registry.exportimport.handler, line 49, in importRegistry
  Module plone.app.registry.exportimport.handler, line 92, in importDocument
  Module plone.app.registry.exportimport.handler, line 212, in importRecord
ValueError: Cannot find a field for the record plone.app.tiles. Add a                 <field /> element or reference an interface and field name.
hvelarde commented 9 years ago

we need to add the tile to the registry programmatically instead of using GenericSetup.

idgserpro commented 9 years ago

Programatically, I think the best way is to do a try except for plone.app.tiles in setuphandlers.py, and, if available, you add the tile to registry.

Or create a new profile and put a zcml condition installed plone.app.tiles, so it will only appear to be installed if plone.app.tiles is installed. It will be a similar behavior like plone.app.contenttypes, it will appear on controlpanel "Add tiles support for collective.nitf".

Or a mix of both, create a new profile with the zcml condition and, in setuphandlers, try except for plone.app.tiles and if available apply the profile.

This is indeed a problem. The faster way is to just add plone.app.tiles as dependency, but since the tiles in collective.nitf are optional...

hvelarde commented 9 years ago

IMO, that's over-engineered and needless; there's no harm on leaving an empty registry record for plone.app.tiles on uninstalling.

but that's only my opinion :cactus:

idgserpro commented 9 years ago

Nah, you're right if there's no harm. I thought about the try except just in a caution manner, I don't have that much experience with empty records in plone.app.registry.

And all the profile suggestions were just a brainstorm though, to test how insane we can get.

hvelarde commented 9 years ago

we can fix it more easily by removing the record in Install.py if it's empty.