ipol-journal / ipolDevel

IPOL demo system development
GNU Affero General Public License v3.0
23 stars 3 forks source link

Refactor/merge conversion into core #137

Closed kidanger closed 1 year ago

kidanger commented 1 year ago

This moves the conversion service into a module for core.

The function Conversion.convert got a major cleanup, please review carefully:

mcolom commented 1 year ago

Thanks for this contribution. For the moment I agree with these large changes, but it'd be better to discuss it in the team before actually writing code.

I have two questions: 1) The service is now /api/core/convert_tiff_to_png. We should keep the old one, even if executed inside Core: /api/conversion/convert_tiff_to_png, right?

2) Is any other component of the system using the conversion, expect from Core? If so, it's good to keep the service in the API. Otherwise, Core can just import the module and run its functions without the API. It's possible the Archive wants to create thumbs with the API, but it could be that it simply imports the corresponding function directly. It's something to check.

kidanger commented 1 year ago
  1. Tne endpoint /api/conversion/convert_tiff_to_png didn't change. It was renamed in one of the commits, then fixed back thanks to the cherrypy.tree.mount(core.get_conversion_api(), '/api/conversion', ...).
  2. This endpoint is used (only) by demo.upload.js so it needs to stay as an API for now.
mcolom commented 1 year ago

Thanks for the answers. Approved!