search

databacker / mysql-backup

image to enable automated backups of mysql databases in containers
636 stars 178 forks source link

Post backup scripts in v1.0 #323

Closed Dachande663 closed 2 months ago

Dachande663 commented 2 months ago

Hi,

We recently updated to the RCs of v1 and I can't seem to get post backup scripts to execute anymore.

Looking at dump.go, I'm not sure if it's a typo, but the DumpOptions PostBackupScripts field is been set to the same value as pre backup. Is this right?

https://github.com/databacker/mysql-backup/blob/1682a343c3c85caa42b26e92da7dc42baf99c10b/cmd/dump.go#L182

Thanks.

deitch commented 2 months ago

I appreciate the civility of, "I'm not sure if it's a typo". 😁 It most certainly is a typo, and thank you for catching it.

Since you are comfortable with go, did you try running it with that changed and see if it resolves your issue? Obviously will open a PR to fix that, but it would be good to understand if that alone is your issue, or something else.

deitch commented 2 months ago

I wondered why the integration tests didn't catch it. I just realized why. The integration tests do not run the actual command-line executable. Instead they call the library that is called from the CLI, setting the parameters explicitly. It is much more difficult to write and manage tests without that.

There is a separate set (within the unit tests) that check if parameters get passed properly to those library calls, but that specifically parameter is not exercised. We shall fix that.

Dachande663 commented 2 months ago

Hi, I've been running the docker images so haven't actually needed the go build chain just yet. I'm happy to make a PR with that change.

There is another issue where AWS_ENDPOINT_URL doesn't seem to be respected if in an ENV var vs passed as --aws-endpoint-url, but I didn't want to muddy the issue. Could that be a similar omission in config handling?

Edit: and thank you for the super quick reply!