Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ensure jm working dir exists #68

Merged
merged 3 commits into from
Oct 18, 2022
Merged

Conversation

theborakompanioni
Copy link
Collaborator

@theborakompanioni theborakompanioni commented Oct 13, 2022

Fix for missing jm working directory if it is not mapped to the host system.

The log feature introduced a regression bug, when the logs moved from directory $JM_WORK_DIR/logs to /var/log/jam.
Since then, it was not ensured that the directory actually exists.

In most setups this is no problem, as the directory will most probably always be mapped to the host system via --volume jmdatadir:/root/.joinmarket, but in some situation, e.g. just starting the image once, this might lead to an error when the config file is copied: cp: cannot create regular file '/root/.joinmarket/joinmarket.cfg': No such file or directory.

The added line mkdir --parents "${DATADIR}/" now creates the directory in case it is missing.

Small additional change: In order to prevent supervisord from searching for its configuration file in multiple locations and printing a warning message to stdout, the path to the config file is passed as a parameter.

@theborakompanioni theborakompanioni added the bug Something isn't working label Oct 13, 2022
@theborakompanioni theborakompanioni self-assigned this Oct 13, 2022
@theborakompanioni theborakompanioni marked this pull request as ready for review October 13, 2022 14:18
@theborakompanioni
Copy link
Collaborator Author

Not totally sure if this is the right approach, to be honest. Maybe instead of creating the dir, we should exit with a more descriptive error indicating that the directory should be mapped to the host system? Let me know what you think @dergigi

@dergigi
Copy link
Contributor

dergigi commented Oct 18, 2022

I think creating the directory is fine. There shouldn't be any bad side-effects to this, and personally I prefer fixing things that can be fixed easily as opposed to throwing errors for things that the user should not have to care about.

Copy link
Contributor

@dergigi dergigi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, feel free to merge ✅

@theborakompanioni theborakompanioni merged commit d1dd634 into master Oct 18, 2022
@theborakompanioni theborakompanioni deleted the mkdir-datadir branch October 18, 2022 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants