kartoza / docker-pg-backup

A cron job that will back up databases running in a docker postgres container
GNU General Public License v2.0
452 stars 103 forks source link

Incorrect env variables names #18

Closed androane closed 4 years ago

androane commented 5 years ago

pgenv.sh has

export PGUSER=docker
export PGPASSWORD=docker
export PGPORT=5432
export PGHOST=db
export PGDATABASE=gis
export DUMPPREFIX=PG
export ARCHIVE_FILENAME=

while the documentation has other variable names

NyakudyaA commented 5 years ago

@androane look at start.sh you will see the environment variables being used there

androane commented 5 years ago

@NyakudyaA yes. What I'm trying to say is that there's a mismatch between those names and the ones in the doc (ex PGUSER vs POSTGRES_USER)

NyakudyaA commented 5 years ago

@androane I do not see them perhaps you can point me on the correct line where there is a mismatch. The best way for you would be to fix the issues and we can look at your PR and happily merge

androane commented 5 years ago

Depending on what you want to change. The docs or the basecode :)

Readme: https://github.com/kartoza/docker-pg-backup#specifying-environment In start.sh: https://github.com/kartoza/docker-pg-backup/blob/master/start.sh#L42

Note that the ones in start.sh are like PGUSER. In the README, there's POSTGRES_USER, which is not being used at all in start.sh.

NyakudyaA commented 5 years ago

@androane You can change the script start.sh to match the environment variables in the doc. Thanks for picking it up

androane commented 5 years ago

@NyakudyaA why not the other way around?

NyakudyaA commented 5 years ago

@androane Because this image inherits from the kartoza/postgis:version so anything changed should apply directly to upstream images

androane commented 5 years ago

@NyakudyaA can't push branch because of permissions. Would like to open a PR

NyakudyaA commented 5 years ago

@androane just do a fork of this repo, do your work and then do a PR against this branch and I will review it and merge

androane commented 5 years ago

@NyakudyaA would like to create a PR, but can't push my local branch

NyakudyaA commented 5 years ago

@androane give me the URL of your branch

androane commented 5 years ago

@NyakudyaA are we talking different languages? I cloned the repo. I created a local new branch. I did the changes. Now I'm trying to push the branch to your repo so I can open a PR. But I don't have permission. So what URL do you want ??

NyakudyaA commented 5 years ago

@androane That is not how you should be doing it.

androane commented 5 years ago

Done

NyakudyaA commented 5 years ago

thanks @androane I will take a look and test

androane commented 5 years ago

@NyakudyaA will this be done soon? I need it and I was wondering if I should use my fork or wait for your update. Thanks!

NyakudyaA commented 5 years ago

Hi @androane I will merge soon and push a new image

androane commented 5 years ago

@NyakudyaA when I pull your image from dockerhub, it's still not working. Perhaps that image is not updated? It's looking at all PG prefixes env variables, instead of POSTGRES_. Haven't tried yet, but most probably it works if I ony clone the repo due to the latest fix

NyakudyaA commented 5 years ago

@androane You can clone the repo and build it , I am still waiting for permission to push to that specific repo in docker hub

androane commented 5 years ago

any news on updating the image in dockerhub? I'd really need it :)

NyakudyaA commented 4 years ago

Done in dockerhub