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

Readme #451

Merged
merged 6 commits into from
Jun 3, 2022
Merged

Readme #451

merged 6 commits into from
Jun 3, 2022

Conversation

eliwaksbaum
Copy link
Contributor

fixes #446

@napulen napulen marked this pull request as ready for review June 2, 2022 20:07
Copy link
Member

@napulen napulen left a comment

Choose a reason for hiding this comment

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

This is great! Thank you @eliwaksbaum.

I added some minor changes.

@napulen
Copy link
Member

napulen commented Jun 2, 2022

I'll re-read before merging it.

@dchiller
Copy link
Collaborator

dchiller commented Jun 2, 2022

What's the change to the nginx part of docker-compose.yaml?

Copy link
Collaborator

@dchiller dchiller left a comment

Choose a reason for hiding this comment

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

Love the readme!

@dchiller
Copy link
Collaborator

dchiller commented Jun 2, 2022

Maybe one thing to add to the read-me is about that error where (if you are developing) you need to make sure the folders where the postgres db is persisted are deleted -- or else upon rebuild postgres will not initialize....

@napulen
Copy link
Member

napulen commented Jun 3, 2022

Maybe one thing to add to the read-me is about that error where (if you are developing) you need to make sure the folders where the postgres db is persisted are deleted -- or else upon rebuild postgres will not initialize....

Yes, that's important

@eliwaksbaum
Copy link
Contributor Author

Maybe one thing to add to the read-me is about that error where (if you are developing) you need to make sure the folders where the postgres db is persisted are deleted -- or else upon rebuild postgres will not initialize....

That happens when you change the password, right?

@napulen
Copy link
Member

napulen commented Jun 3, 2022

Maybe one thing to add to the read-me is about that error where (if you are developing) you need to make sure the folders where the postgres db is persisted are deleted -- or else upon rebuild postgres will not initialize....

That happens when you change the password, right?

Yeah, I think the particular case I ran into was

  • I forgot to set up the password on .env and launched the container
  • Realized that I didn't set up a password, so I edited .env
  • I re-built the containers and ran into this postgres issue

@napulen
Copy link
Member

napulen commented Jun 3, 2022

I pushed a rough draft of the postgres issue

@napulen
Copy link
Member

napulen commented Jun 3, 2022

I also re-read the complete README file and I am good to go.

Can you review/double-check/edit the postgres section? After that, I think we are good to go with this documentation 👍

@eliwaksbaum
Copy link
Contributor Author

Is removing the volume with the docker cli enough or do you have to actually delete the files on your machine?

@napulen
Copy link
Member

napulen commented Jun 3, 2022

Is removing the volume with the docker cli enough or do you have to actually delete the files on your machine?

I don't know for sure, but I can just add the rm instruction to the readme.

@eliwaksbaum
Copy link
Contributor Author

Ok looks good to me!

@dchiller
Copy link
Collaborator

dchiller commented Jun 3, 2022

I have always just removed the files, independent of docker, but if removing the volume does the trick, looks great!

@napulen
Copy link
Member

napulen commented Jun 3, 2022

The problem I had removing just the files is that I run into a weird permission denied issue, probably because something on the docker daemon is holding that folder.

@napulen
Copy link
Member

napulen commented Jun 3, 2022

Ok, merging the current docs. Corrections can come in future PRs as usual.

@napulen napulen merged commit aaae06e into DDMAL:main Jun 3, 2022
@eliwaksbaum eliwaksbaum deleted the readme branch June 3, 2022 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update documentation for docker version
3 participants