-
Notifications
You must be signed in to change notification settings - Fork 1
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
GH-8 Add dump_only_these_tables input #9
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mmatsoo I see you still pushing work to this. I'm guessing there are bugs to work out still? Anything I can help with?
@adamzimmermann This was working fine, locally, but
In short, whatever works locally is not liked by |
@mmatsoo can you revert it to the working state or point me to the commit and then share what shell check is barking about (I guess I can see it in the check output)? We could disable shell-check for the troublesome line if we are really stuck. 🙁 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While it would be preferable to pass the list of tables as a list rather than string, this works and is good enough and has the virtue of simplicity. Nice work.
do | ||
# Add table options into array. | ||
DUMP_ONLY_THESE_TABLES+=("--table $table") | ||
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really cool use of shell "arrays" here with [*] below.... That was really smart.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think "smart" is really just finding something to copy/paste from StackOverflow.
I guess the only thing remaining here is make shellcheck shut up. I assume this does not work?:
In my shell(s) it works (both in zsh and then in bash and in sh). Herewith my sh session:
That's the only blocker here. We need to get shellcheck to pass this. If my suggestion doesn't work, I wonder if we can remove the |
@apotek FWIW - the shebang is from what we use in the Copy/paste from documentation page
|
In my testing, this worked
even with no change to the shebang. |
Description
@markdorison @adamzimmermann This PR adds an optional input,
dump_only_these_tables
that can take an arbitrary list of table names, separated by spaces.The
entrypoint.sh
script loops through the list of table names and concatenates them into theplatform db:dump
command. If the input variable is missing or empty, the script does not mind and continues to run, dumping the entire DB, as expected.Testing
Tested locally.
Related Ticket