databacker / mysql-backup

image to enable automated backups of mysql databases in containers
648 stars 184 forks source link

Documentation inconsistencies #287

Closed Ramblurr closed 7 months ago

Ramblurr commented 7 months ago

There are a few inconsistencies in the documentation vs what the go-lang mysql-backup actually implements.

COMPRESSION

https://github.com/databacker/mysql-backup/blob/ea1580bf84037991162969b62c66c7d93ddc7a7f/docs/configuration.md?plain=1#L87

TMP_PATH

https://github.com/databacker/mysql-backup/blob/ea1580bf84037991162969b62c66c7d93ddc7a7f/docs/configuration.md?plain=1#L89

NICE

https://github.com/databacker/mysql-backup/blob/ea1580bf84037991162969b62c66c7d93ddc7a7f/docs/configuration.md?plain=1#L89

Other options in docs/configuration.md are missing the DB_DUMP_ prefix

...but not all of them need the prefix, for example the smb related options don't seem to require the prefix.

deitch commented 7 months ago

This should be named DB_DUMP_COMPRESSION

Good catch. PR for docs coming soon.

the golang mysql-backup only supports gzip, not bzip2

Oops. It should. And golang has native bzip2 support. Rather than changing the docs, do you mind opening a separate issue on missing bzip2 support?

the golang mysql-backup doesn't actually use this parameter (personally I don't think it's needed)

I agree, so let's just remove it.

the golang mysql-backup doesn't actually use this parameter NICE

Correct. This should be added back. Do you mind opening a separate issue? This is just for the entrypoint used in the container image.

the golang mysql-backup seems to expect all env vars to be prefixed with DBDUMP but not all of them need the prefix, for example the smb related options don't seem to require the prefix.

Right. Tried to get everything that is unique to mysql-backup to be DB_DUMP_*, while other things that are not - like AWS or smb "standard" ones - to be their native.

Are you saying that they expect the prefix for some and shouldn't? Or they do not expect the prefix and shouldn't, and just he docs are off?

Ramblurr commented 7 months ago

Created #290 and #291.

Thanks for updating the docs!

Are you saying that they expect the prefix for some and shouldn't? Or they do not expect the prefix and shouldn't, and just he docs are off?

I think it is fine how it is. There is this line in the docs "Environment variables are all uppercase, with words separated by underscores, and most start with DB_DUMP. " and when I first read it my brain just parsed it as "all start with DB_DUMP", so my fault for not reading.

I'll close this issue! Thanks for the feedback.