changes for improved kitodo docker compose #23
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bertsky
requested changes
Jun 8, 2022
There was a problem hiding this comment.
Great stuff. (But the Makefile and Readme must be adapted, too.)
IMO it makes sense to keep docker-compose.kitodo-app.yml separate for 2 reasons:
- we want to switch between
managedandstandaloneindependent of the Kitodo services - perhaps someone wants to run without the Kitodo services (by not adding that to
COMPOSE_FILE)
The latter could even be supported in the makefile with a MODE choice or similar.
(For example, MODE=native for native Kitodo installation but Dockerized Manager and Controller. Or we make it more clear by renaming: CONTROLLER=managed vs CONTROLLER=standalone and PRODUCTION=managed vs PRODUCTION=standalone.)
…production_ocrd into changes_for_kitodo
bertsky
reviewed
Jun 14, 2022
.env
Outdated
| APP_BUILD_RESOURCES=${APP_BUILD_CONTEXT}/build-resources | ||
| APP_BUILD_RESOURCES=./build-resources |
There was a problem hiding this comment.
Why even remove APP_BUILD_CONTEXT here, how can that work?
There was a problem hiding this comment.
Cause of APP_BUILD_CONTEXT we are already in context and we can only build image below this context.
There was a problem hiding this comment.
Cause of APP_BUILD_CONTEXT we are already in context and we can only build image below this context.
I don't understand that, grammatically.
There was a problem hiding this comment.
Cause of the APP_BUILD_CONTEXT env we are already in the directory of Dockerfile to build image of resources. In order not to break out of it e.g. by change into a parent directory with absolute paths, I set here the APP_BUILD_RESOURCES relatively.
bertsky
reviewed
Jun 14, 2022
|
|
||
| ./kitodo: ./_resources/config_modules.zip | ||
|
|
||
| ./kitodo: ./_resources/config_modules.zip |
There was a problem hiding this comment.
Still hesitant to completely skip telling make about ./_resources/config_modules.zip – but if its just a provisional state, ok.
…docker, ajust docker compose to use new overwrites directory provided by the kitodo docker image and improve makefile to handle new data structure
|
Branch includes a new structuring of the sample data and an implementation to make them available in Kitodo.Production now. |
|
I need https://github.com/markusweigelt/kitodo-production-docker/pull/10 to start up without errors, but still when I enter the App URL I end up on |
…production_ocrd into changes_for_kitodo
…production_ocrd into changes_for_kitodo
|
With the current version I get a new problem when issueing the OCR task: result page with stack trace
|
|
@bertsky logs before traceback
|
|
@bertsky Sorry for the last change. I'll try to pay more attention to .env changes. |
|
If you update the Manager (after merging slub/ocrd_manager#25) and Controller (with latest master), then everything should work again. (With logging now mirrored between Controller and Manager, and workspace names independent between Controller and Manager.) |
This was
linked to
issues
Jun 24, 2022
bertsky
reviewed
Jun 24, 2022
Makefile
Outdated
Comment on lines
9
to
14
| ifeq ($(shell test -e .env.local && echo -n yes),yes) | ||
| COMPOSE_ENV_FILE ?= .env.local | ||
| else | ||
| COMPOSE_ENV_FILE ?= .env | ||
| endif | ||
|
|
There was a problem hiding this comment.
Less verbose version:
Suggested change
| ifeq ($(shell test -e .env.local && echo -n yes),yes) | |
| COMPOSE_ENV_FILE ?= .env.local | |
| else | |
| COMPOSE_ENV_FILE ?= .env | |
| endif | |
| COMPOSE_ENV_FILE = $(firstword $(wildcard .env.local .env)) |
|
@bertsky Still made the changes discussed. In the docker-compose for the manager I added an environment variable MANAGER_CONTROLLER_PORT_SSH to distinguish the container port from the host port cause of conflict in |
@markusweigelt I don't understand that. Where is this variable used/referenced? Why not just use |
@bertsky |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.