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

(MODULES-11251) Add support for backup provider "pg_dump" #1319

Merged
merged 1 commit into from Aug 17, 2022
Merged

(MODULES-11251) Add support for backup provider "pg_dump" #1319

merged 1 commit into from Aug 17, 2022

Conversation

fraenki
Copy link
Contributor

@fraenki fraenki commented Jan 11, 2022

This adds support for backup "providers" to puppetlabs/postgresql. The feature is heavily inspired by a similar feature in puppetlabs/mysql.

See Puppet ticket for full details:
https://tickets.puppetlabs.com/browse/MODULES-11251

@CLAassistant
Copy link

CLAassistant commented Jan 11, 2022

CLA assistant check
All committers have signed the CLA.

@puppet-community-rangefinder
Copy link

postgresql::backup::pg_dump is a class

that may have no external impact to Forge modules.

postgresql::params is a class

that may have no external impact to Forge modules.

postgresql::server is a class

that may have no external impact to Forge modules.

This module is declared in 70 of 578 indexed public Puppetfiles.


These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

@fraenki fraenki changed the title Add support for backup provider "pg_dump" (MODULES-11251) Add support for backup provider "pg_dump" Jan 11, 2022
@fraenki fraenki marked this pull request as ready for review January 11, 2022 13:29
@fraenki fraenki requested a review from a team as a code owner January 11, 2022 13:29
@bastelfreak
Copy link
Collaborator

Hi,
I like the idea and added some inline comments. I want to mention that Vox Pupuli also has a module to do database dumps: https://forge.puppet.com/modules/puppet/dbbackup#table-of-contents

@fraenki
Copy link
Contributor Author

fraenki commented Jan 11, 2022

Hi, I like the idea and added some inline comments. I want to mention that Vox Pupuli also has a module to do database dumps: https://forge.puppet.com/modules/puppet/dbbackup#table-of-contents

Thanks for the review. Actually I've already had a look at puppet/dbbackup, but I really like the integrated approach of puppetlabs/mysql's backup feature, and I cannot use systemd, which lead to the creation of this PR. :)

@fraenki
Copy link
Contributor Author

fraenki commented Jan 12, 2022

I've added some tests with the latest commit.

@github-actions
Copy link

github-actions bot commented May 7, 2022

This PR has been marked as stale because it has been open for a while and has had no recent activity. If this PR is still important to you please drop a comment below and we will add this to our backlog to complete. Otherwise, it will be closed in 7 days.

@github-actions github-actions bot added the stale label May 7, 2022
@github-actions github-actions bot closed this May 14, 2022
@bastelfreak bastelfreak reopened this May 14, 2022
@puppet-community-rangefinder
Copy link

postgresql::backup::pg_dump is a class

that may have no external impact to Forge modules.

postgresql::params is a class

Breaking changes to this file WILL impact these 2 modules (exact match):
Breaking changes to this file MAY impact these 3 modules (near match):

postgresql::server is a class

Breaking changes to this file WILL impact these 38 modules (exact match):
Breaking changes to this file MAY impact these 16 modules (near match):

This module is declared in 70 of 579 indexed public Puppetfiles.


These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

@bastelfreak
Copy link
Collaborator

dear puppet, please do not autoclose PR that are only stale because nine of you reviewed it...

@fraenki sadly this now needs a rebase, could you please do that?

@github-actions github-actions bot removed the stale label May 15, 2022
@github-actions
Copy link

Hello! 👋

This pull request has been open for a while and has had no recent activity. We've labelled it with attention-needed so that we can get a clear view of which PRs need our attention.

If you are waiting on a response from us we will try and address your comments on a future Community Day.

Alternatively, if it is no longer relevant to you please close the PR with a comment.

Please note that if a pull request receives no update for 7 after it has been labelled, it will be closed. We are always happy to re-open pull request if they have been closed in error.

@fraenki
Copy link
Contributor Author

fraenki commented Jul 19, 2022

PR needs a review, I'll rebase ASAP...

@chelnak
Copy link
Contributor

chelnak commented Jul 19, 2022

hey @fraenki ! Appreciated!

@fraenki
Copy link
Contributor Author

fraenki commented Jul 26, 2022

@chelnak Rebased. Ready for review :)

@chelnak
Copy link
Contributor

chelnak commented Jul 26, 2022

Hey @fraenki ! I'm on PTO this week so pinging @pmcmaw and @david22swan for assistance!

@david22swan
Copy link
Member

@fraenki Look's like your getting a few spec test failures.

@fraenki
Copy link
Contributor Author

fraenki commented Aug 1, 2022

I've force-pushed a fix, please re-run the spec tests.

@fraenki
Copy link
Contributor Author

fraenki commented Aug 1, 2022

Since I'm unable to run the tests locally, I need to request a re-run of the spec tests again... :-/

@fraenki
Copy link
Contributor Author

fraenki commented Aug 1, 2022

The spec tests are finally passing. 🎉
(However, some tests seem to timeout, which is also the case for the nightly tests.)

@fraenki
Copy link
Contributor Author

fraenki commented Aug 17, 2022

@chelnak @david22swan What do you think? Can this PR be merged? :)

Copy link
Member

@david22swan david22swan left a comment

Choose a reason for hiding this comment

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

LGTM

Apologies for the wait on this.
Anyway it look's good to me so I'm gonna go ahead and merge :)
Thank's for putting in the work

@david22swan david22swan merged commit f8e2568 into puppetlabs:main Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants