Add mongodb dump hook #483

Merged
witten merged 3 commits from sanzoghenzo/borgmatic:mongodb_hook into master 2022-01-04 23:50:26 +00:00
Contributor

Merry Christmas!

For the holydays I tried to solve issue #288 with this MR.

I was in a hurry and only tested with a single database and basic authentication (username and password).

I just copied and adapted the postgresql unit tests, and they all pass.

Merry Christmas! For the holydays I tried to solve issue #288 with this MR. I was in a hurry and only tested with a single database and basic authentication (username and password). I just copied and adapted the postgresql unit tests, and they all pass.
sanzoghenzo added 1 commit 2021-12-26 00:10:37 +00:00
continuous-integration/drone/pr Build is failing Details
6b7653484b
Add mongodb dump hook
witten requested changes 2021-12-27 23:36:20 +00:00
witten left a comment
Owner

This looks awesome! Thank you so much for taking the time to implement MongoDB support.

It appears that there is one test failing however, at least in CI: https://build.torsion.org/borgmatic-collective/borgmatic/111/1/4

Specifically, it's the test_database_dump_and_restore_with_directory_format test. You should be able to reproduce this locally with scripts/run-full-dev-tests, assuming you have Docker and Compose available locally.

The error in question is /bin/sh: mongodump: not found, which to me suggests that you just need the MongoDB client installed in the test image. The place to do that would be in scripts/run-full-tests. You might be able to just add mongodb-tools to the first apk add line.

Additionally, you'll probably need to add a MongoDB server image to tests/end-to-end/docker-compose.yaml and .drone.yml so that a MongoDB server is available both for local tests and CI tests (respectively).

Let me know if you need any help with this!

This looks awesome! Thank you so much for taking the time to implement MongoDB support. It appears that there is one test failing however, at least in CI: https://build.torsion.org/borgmatic-collective/borgmatic/111/1/4 Specifically, it's the `test_database_dump_and_restore_with_directory_format` test. You should be able to reproduce this locally with `scripts/run-full-dev-tests`, assuming you have Docker and Compose available locally. The error in question is `/bin/sh: mongodump: not found`, which to me suggests that you just need the MongoDB client installed in the test image. The place to do that would be in `scripts/run-full-tests`. You might be able to just add `mongodb-tools` to the first `apk add` line. Additionally, you'll probably need to add a MongoDB server image to `tests/end-to-end/docker-compose.yaml` and `.drone.yml` so that a MongoDB server is available both for local tests and CI tests (respectively). Let me know if you need any help with this!
@ -776,0 +813,4 @@
example: trustsome1
format:
type: string
enum: ['archive', 'directory']
Owner

This doesn't mention custom as one of the options, but it looks like the default is custom below. Should it be here too?

This doesn't mention `custom` as one of the options, but it looks like the default is `custom` below. Should it be here too?
sanzoghenzo marked this conversation as resolved
@ -776,0 +817,4 @@
description: |
Database dump output format. One of "archive",
or "directory". Defaults to "archive" (unlike
raw pg_dump). See pg_dump documentation for
Owner

This references pg_dump. Should it refer to something Mongo-specific instead?

This references `pg_dump`. Should it refer to something Mongo-specific instead?
sanzoghenzo marked this conversation as resolved
sanzoghenzo added 1 commit 2021-12-29 21:20:07 +00:00
continuous-integration/drone/pr Build is failing Details
7c6ce9399c
fix integration tests and mongodb auth
Author
Contributor

Sorry for not running the integration test sooner!

I fixed it, and added the "auth_db" option to specify a diffrerent authentication database than the one to be exported.

There are plenty of other options that I didn't expose (different auth mechanisms, ssl certs and so on) because I don't use them (yet), but for now one can simply use the "options" item.

Sorry for not running the integration test sooner! I fixed it, and added the "auth_db" option to specify a diffrerent authentication database than the one to be exported. There are plenty of other options that I didn't expose (different auth mechanisms, ssl certs and so on) because I don't use them (yet), but for now one can simply use the "options" item.
Owner

Thanks, this is great! Looks like the continuous integration is failing on some versions of Alpine but not others. What I've done is drop support for Python 3.6 (and the corresponding version of Alpine) in master, so that eliminates one failure. And then I've also dropped the Python 3.7 build just because (as that version of Alpine doesn't even include a Mongo client). So I think your build should pass once all that's merged in.

Thanks, this is great! Looks like the continuous integration is failing on some versions of Alpine but not others. What I've done is drop support for Python 3.6 (and the corresponding version of Alpine) in master, so that eliminates one failure. And then I've also dropped the Python 3.7 build just because (as that version of Alpine doesn't even include a Mongo client). So I think your build should pass once all that's merged in.
sanzoghenzo added 1 commit 2022-01-04 21:22:23 +00:00
continuous-integration/drone/pr Build is passing Details
87001337b4
Merge master into mongodb_hook
Author
Contributor

Sorry, I realized that I needed to merge your upstream changes to my branch!

It works now!

Sorry, I realized that I needed to merge your upstream changes to my branch! It works now!
Owner

Cool.. Thank you! Heads up that I might rename the new auth_db option for consistency with some of the other options prior to release.

Cool.. Thank you! Heads up that I might rename the new `auth_db` option for consistency with some of the other options prior to release.
witten merged commit 07d7ae60d5 into master 2022-01-04 23:50:26 +00:00
Owner

Just released in borgmatic 1.5.22! FYI I did end up renaming auth_db to authentication_database.

Just released in borgmatic 1.5.22! FYI I did end up renaming `auth_db` to `authentication_database`.
sanzoghenzo deleted branch mongodb_hook 2022-01-05 23:01:56 +00:00
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: borgmatic-collective/borgmatic#483
No description provided.