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

(SUP-2372) add pg_repack schema reset task #53

Merged
merged 5 commits into from
May 19, 2021

Conversation

pgrant87
Copy link
Contributor

@pgrant87 pgrant87 commented May 4, 2021

Task added to reset pg_repack schema

@pgrant87 pgrant87 requested a review from MartyEwings May 4, 2021 17:27
@pgrant87 pgrant87 requested a review from a team as a code owner May 4, 2021 17:27
Copy link
Contributor

@MartyEwings MartyEwings left a comment

Choose a reason for hiding this comment

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

Would it be possible to have exception handling when not running on a Primary Postgres DB?

@pgrant87 pgrant87 requested a review from MartyEwings May 7, 2021 17:02
@MartyEwings MartyEwings changed the title (SUP-2372) (SUP-2372) add pg_repack schema fox task May 7, 2021
@MartyEwings MartyEwings changed the title (SUP-2372) add pg_repack schema fox task (SUP-2372) add pg_repack schema reset task May 10, 2021
@m0dular m0dular self-requested a review May 11, 2021 01:39
@m0dular
Copy link
Contributor

m0dular commented May 11, 2021

I looked at the enterprise_tasks code, and I think the approach here of checking if the pe-postgresql service is present is good. That's basically what we do to check for postgres versions in that code. So my feedback is basically opinions about bash tasks in general.

I wanted to turn this code into a bash helper for Bolt, but that hasn't happened as of yet. The main thing it gets you is error handling; specifically, it captures stderr to a temp file and returns it in a json structure if you call the fail function. Something a lot of shell tasks lack is returning a useful error message if something fails because you have to do it yourself. IMO I would like to see all of our tooling standardize on this, but we can discuss this. FWIW, ca_extend, facts, service, and package all use it.

@MartyEwings
Copy link
Contributor

@m0dular so from my understanding, we package the common.sh, and update the .json, declare and source it at the top of the bash script, change all variables in the main script from PT_variable to just variable, and call fail or success instead of the various Echos?

@m0dular
Copy link
Contributor

m0dular commented May 11, 2021

That's basically it! A metadata example is here. I think that task is a good example of how to use fail and success. We also need to source and declare it that way to keep shellcheck happy.

@MartyEwings
Copy link
Contributor

@m0dular How does that look

@MartyEwings
Copy link
Contributor

I have tested this task and it works for me

@MartyEwings MartyEwings merged commit 406df3c into puppetlabs:main May 19, 2021
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.

3 participants