jbaiter / demetsiiify

Web service for creating and hosting IIIF manifests from METS/MODS documents
https://demetsiiify.jbaiter.de
GNU Affero General Public License v3.0
34 stars 6 forks source link

Latest code not working (undefined variables, wrong function call) #7

Open stweil opened 6 years ago

stweil commented 6 years ago

The code uses undefined variables mets_doc and manifest_ident and calls _fill_manifest_metadata with only one instead of two arguments.

I had to fix those lines to get something which works partially:

diff --git a/demetsiiify/iiif.py b/demetsiiify/iiif.py
index 7b861db..1aeafc5 100644
--- a/demetsiiify/iiif.py
+++ b/demetsiiify/iiif.py
@@ -175,7 +175,7 @@ def _make_empty_manifest(ident, label):
         current_app.config['SERVER_NAME']))
     manifest_factory.set_iiif_image_info('2.0', 0)
     manifest = manifest_factory.manifest(ident=manifest_ident,
-                                         label=make_label(mets_doc.metadata))
+                                         label=label)
     return manifest

@@ -212,9 +212,12 @@ def make_manifest(ident, mets_doc, physical_map, thumbs_map):
     :returns:               Generated IIIF manifest
     :rtype:                 dict
     """
+    manifest_ident = '{}://{}/iiif/{}/manifest'.format(
+        current_app.config['PREFERRED_URL_SCHEME'],
+        current_app.config['SERVER_NAME'], ident)
     manifest = _make_empty_manifest(ident=manifest_ident,
                                     label=make_label(mets_doc.metadata))
-    _fill_manifest_metadata(manifest)
+    _fill_manifest_metadata(manifest, mets_doc.metadata)

     phys_to_canvas = {}
     seq = manifest.sequence(ident='default')
jbaiter commented 6 years ago

Hey Stefan, thank you for pointing that out. I'm away this weekend, but will give the code a thorough look next week, including #8 and any other issues you might find :-)

stweil commented 6 years ago

This patch works better:

diff --git a/demetsiiify/iiif.py b/demetsiiify/iiif.py
index 7b861db..63dc9f6 100644
--- a/demetsiiify/iiif.py
+++ b/demetsiiify/iiif.py
@@ -174,8 +174,7 @@ def _make_empty_manifest(ident, label):
         current_app.config['PREFERRED_URL_SCHEME'],
         current_app.config['SERVER_NAME']))
     manifest_factory.set_iiif_image_info('2.0', 0)
-    manifest = manifest_factory.manifest(ident=manifest_ident,
-                                         label=make_label(mets_doc.metadata))
+    manifest = manifest_factory.manifest(ident=manifest_ident, label=label)
     return manifest

@@ -212,9 +211,9 @@ def make_manifest(ident, mets_doc, physical_map, thumbs_map):
     :returns:               Generated IIIF manifest
     :rtype:                 dict
     """
-    manifest = _make_empty_manifest(ident=manifest_ident,
+    manifest = _make_empty_manifest(ident=ident,
                                     label=make_label(mets_doc.metadata))
-    _fill_manifest_metadata(manifest)
+    _fill_manifest_metadata(manifest, mets_doc.metadata)

     phys_to_canvas = {}
     seq = manifest.sequence(ident='default')
jbaiter commented 6 years ago

I'm currently refactoring the code quite a bit, so I can finally add some unit tests. In its current state, the whole logic is horribly coupled and very convoluted in some places, it's one of those "what was I thinking??" situations :-) expect an update by the weekend.

stweil commented 6 years ago

Great! I promise to give it a try then.

stweil commented 6 years ago

Thank you for pushing the refactored code. It still does not work for me.

Error: class uri 'gevent' invalid or not found:
[...]
ModuleNotFoundError: No module named 'greenlet'
jbaiter commented 6 years ago

Did you try the Docker-based setup as per the instructions in the README? Should be working now, seems like you need to explicitely add the greenlet dependency :thinking:

stweil commented 6 years ago

Thank you. That's fixed now. This issue remains for the Docker based setup:

$ docker-compose run webapp python manage.py create
Traceback (most recent call last):
  File "manage.py", line 1, in <module>
    from flask import current_app
ModuleNotFoundError: No module named 'flask'
jbaiter commented 6 years ago

Dang, forgot to fix the README :roll_eyes: It should be:

$ docker-compose run webapp pipenv run manage create

I introduced pipenv with the refactor, which takes care of installing all the dependencies in a virtualen environment and automatically load that when executing the program.