artefactual-sdps / enduro

Designed to automate the processing of transfers in multiple Archivematica pipelines.
https://enduro.readthedocs.io/
Apache License 2.0
4 stars 3 forks source link

Enable Archivematica Storage Service AIP downloads #939

Closed djjuhasz closed 1 month ago

djjuhasz commented 1 month ago

Fixes #820.

Enables the Enduro Dashboard "download" button to download AIPs from the Archivematica Storage Service.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 63.44828% with 53 lines in your changes are missing coverage. Please review.

Project coverage is 50.36%. Comparing base (614c4e1) to head (031ab3e).

Files Patch % Lines
cmd/enduro-am-worker/main.go 0.00% 36 Missing :warning:
internal/storage/types/source.go 0.00% 5 Missing :warning:
internal/storage/persistence/ent/client/client.go 77.77% 1 Missing and 3 partials :warning:
internal/workflow/processing.go 83.33% 2 Missing and 2 partials :warning:
cmd/enduro/main.go 0.00% 2 Missing :warning:
cmd/enduro-a3m-worker/main.go 0.00% 1 Missing :warning:
internal/workflow/activities/storage.go 95.83% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #939 +/- ## ========================================== + Coverage 50.05% 50.36% +0.31% ========================================== Files 102 102 Lines 5484 5619 +135 ========================================== + Hits 2745 2830 +85 - Misses 2502 2549 +47 - Partials 237 240 +3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

djjuhasz commented 1 month ago

Testing this requires adding a few new fields to your Archivematica secrets file:

amss_url=https://enduro-temporal.ss.analyst.archivematica.net:8000
amss_user=analyst
amss_api_key=[your_key_here]
jraddaoui commented 1 month ago

Thinking about this and the deployment implications @djjuhasz. Should we try to make everything configurable? Maybe we could add the AMSS location information to a new [am.ss] config section and check its existence on startup, creating it if it doesn't exists. Otherwise, the creation of that location directly in the DB has to be considered on the deployment, while all the other AM options are part of the configuration.

djjuhasz commented 1 month ago

Thinking about this and the deployment implications @djjuhasz. Should we try to make everything configurable? Maybe we could add the AMSS location information to a new [am.ss] config section and check its existence on startup, creating it if it doesn't exists. Otherwise, the creation of that location directly in the DB has to be considered on the deployment, while all the other AM options are part of the configuration.

@jraddaoui I think the downside of putting the full AMSS location info in the config file is then we have the data in two places: in the config, and in the enduro_storage database. Once the AMSS location data is added to the database the data in the config file becomes superfluous.

We need to have the AMSS location data in the database for the Enduro Dashboard "Download AIP" link to work. I think for now it's better to stick with adding the AMSS location data to the enduro_storage database in deployment, like we are doing in the dev environment. If it's a hassle to add the data directly with SQL, we could create a go cmd script to add locations to the database.

I don't think it's ideal to have the AMSSLocationID in the config file really - we may want to consider adding it (along with other config information) to the persistence layer in the future.

jraddaoui commented 1 month ago

@jraddaoui I think the downside of putting the full AMSS location info in the config file is then we have the data in two places: in the config, and in the enduro_storage database. Once the AMSS location data is added to the database the data in the config file becomes superfluous.

I don't think that's an issue, but I can see them updating the configuration at some point. It's not that i like this solution either, just having a lot of conflicts about how tied are those config options in this case. And also about how the entire storage service (from Enduro) works.

We need to have the AMSS location data in the database for the Enduro Dashboard "Download AIP" link to work. I think for now it's better to stick with adding the AMSS location data to the enduro_storage database in deployment, like we are doing in the dev environment. If it's a hassle to add the data directly with SQL, we could create a go cmd script to add locations to the database.

I think it could also be done through the API, but I may be wrong.

I don't think it's ideal to have the AMSSLocationID in the config file really - we may want to consider adding it (along with other config information) to the persistence layer in the future.

I think they are too tied together but then our services are supposed to work independently, that's why I'm having these conflicts. Lets go with what you have done for now, hopefully we can make it clearer/easier in the future.