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

[2242] Improve konduit.sh error handling #376

Merged

Conversation

saliceti
Copy link
Member

@saliceti saliceti commented Feb 11, 2025

Context

The script was not stopping when an error happened, which made it more difficult to troubleshoot errors

Changes proposed in this pull request

Make the script more robust by configuring "set -eu" to stop when there is an error and when a variable is not set

Guidance to review

Try konduit with various commands and arguments:

  • scripts/konduit.sh apply-staging -- psql
  • scripts/konduit.sh apply-staging -- redis-cli
  • scripts/konduit.sh -a apply-review-10343 -- redis-cli
  • scripts/konduit.sh apply-review-10343 -- pg_dump -E utf8 --clean --compress=1 --if-exists --no-owner --verbose --no-password -f bak.gz
  • scripts/konduit.sh -c -i bak.gz apply-review-10343 -- psql
  • scripts/konduit.sh -t 120 -n bat-qa apply-review-10343 -- psql
  • scripts/konduit.sh -k s189t01-gse-stg-inf-kv -d gse-staging get-school-experience-staging -- psql
  • scripts/konduit.sh -k s189t01-gse-stg-inf-kv -s s189t01-gse-stg-pg -d gse-staging get-school-experience-staging -- psql

After merging

Keep an eye on database workflows

Checklist

  • I have performed a self-review of my code, including formatting and typos
  • I have cleaned the commit history
  • I have added the Devops label
  • I have attached the pull request to the trello card

The script was not stopping when an error happened, which made it more
difficult to troubleshoot errors
Make the script more robust by configuring "set -eu" to stop when there
is an error and when a variable is not set
@saliceti saliceti force-pushed the 2242-konduitsh-fail-explicitly-when-permission-is-missing branch from a4c01db to 62278b9 Compare February 11, 2025 12:42
Copy link
Collaborator

@RMcVelia RMcVelia left a comment

Choose a reason for hiding this comment

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

all tests ran as expected

@saliceti saliceti merged commit bd9bd5c into main Feb 11, 2025
6 checks passed
@saliceti saliceti deleted the 2242-konduitsh-fail-explicitly-when-permission-is-missing branch February 11, 2025 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants