Skip to content
This repository has been archived by the owner on Nov 25, 2024. It is now read-only.

Use log directory that we will be able to write to #1799

Merged
merged 1 commit into from
Mar 30, 2021

Conversation

MadLittleMods
Copy link
Contributor

@MadLittleMods MadLittleMods commented Mar 17, 2021

Use log directory that we will be able to write to.

Fix #1644

Pull Request Checklist

  • I have added any new tests that need to pass to sytest-whitelist as specified in docs/sytest.md
  • Pull request includes a sign off

Signed-off-by: Your Name <[email protected]>

@MadLittleMods MadLittleMods force-pushed the eric/1644-permissable-log-directory branch from 3e32073 to 49798cf Compare March 17, 2021 17:52
@MadLittleMods MadLittleMods changed the title Draft: Use log directory that we will be able to write to Use log directory that we will be able to write to Mar 17, 2021
@MadLittleMods MadLittleMods requested a review from kegsay March 17, 2021 23:14
Copy link
Member

@kegsay kegsay left a comment

Choose a reason for hiding this comment

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

Thanks for this! We use this path in numerous other places though, I think I need @neilalexander to help on which need to be changed (the ones for docker are probably fine staying as /var/log:

/cmd/generate-config/main.go:103:				"path": "/var/log/dendrite",
/build/docker/config/dendrite-config.yaml:344:    path: /var/log/dendrite

@kegsay kegsay requested a review from neilalexander March 18, 2021 10:28
@neilalexander
Copy link
Contributor

... which makes me realise that we should probably update the Docker Compose sample files to map the media and log directories somewhere outside of the container.

@MadLittleMods
Copy link
Contributor Author

@kegsay It looks like both of those places are related to the Docker images. generate-config is only used in Docker and isn't documented elsewhere. And the other instance /build/docker/config/dendrite-config.yaml is self-explanatory as Docker.

Anything else stopping this from merging? Seems like we want to iterate a bit on the Docker setup but we can handle in another PR.

@MadLittleMods MadLittleMods requested a review from kegsay March 24, 2021 07:44
@kegsay
Copy link
Member

kegsay commented Mar 30, 2021

Agreed.

@kegsay kegsay merged commit 0ee1c56 into master Mar 30, 2021
@kegsay kegsay deleted the eric/1644-permissable-log-directory branch March 30, 2021 08:53
@MadLittleMods
Copy link
Contributor Author

Thanks for the review @kegsay 🦫

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Log permission denied issue when trying to get started
3 participants