Skip to content

Fixed SQL queries so that they accept table names of only digits.#3

Open
rwoodbury wants to merge 76 commits intoyvoronoy:masterfrom
rwoodbury:master
Open

Fixed SQL queries so that they accept table names of only digits.#3
rwoodbury wants to merge 76 commits intoyvoronoy:masterfrom
rwoodbury:master

Conversation

@rwoodbury
Copy link
Copy Markdown

  • Fixed dissagreement between help statement and actual option usage.
  • Changed handling of configuration file so that values from the config file are always used but are overridden by values from the command line.
  • The added and commented out code is a start at adding/changing the existing table name prefix. Table prefix handling is incomplete.

Tested on Ubuntu 14.04 Server.

- Fixed dissagreement between help statement and actual option usage.
- Changed handling of configuration file so that values from the config file are always used but are overridden by values from the command line.
- The added and commented out code is a start at adding/changing the existing table name prefix. Table prefix handling is incomplete.
@yvoronoy
Copy link
Copy Markdown
Owner

@rwoodbury wow nice job.
Thank you for your huge pull request.
Let me review it and I'll update you soon.

Also please consider for m2 project similar tool https://github.com/yvoronoy/m2install

@rwoodbury
Copy link
Copy Markdown
Author

Thanks! Any commentary, including negative, is welcome. I'm concerned I might have missed some behavior you had in mind.

I will consider M2 installer but it will have to wait until I start working on M2. ;-)

@yvoronoy
Copy link
Copy Markdown
Owner

yvoronoy commented Jun 9, 2016

Sorry for late response @rwoodbury
I little bit tested your changes. Main idea of restore.sh was restore dumps fast as possible, without entering any additional data. And put all required fields to configuration file.

So if I have a dumps, I just run restore.sh it deploy dumps without any additional questions.
I don't want enter every time same information like: base url, db name, user, host etc...
These params calculated dynamically based on current directory
So BASE URL this is = http://host/base_path/ + current_directory/
DB NAME = SOME_PREFIX + current_directory

So I don't wanna think every time how I have to name DB, or what base url should be.
My use case is

  1. create new dir and put dumps.
  2. run restore.sh or restore.sh --force

@rwoodbury
Copy link
Copy Markdown
Author

rwoodbury commented Jun 9, 2016

As far as the parameter handling is concerned I shouldn't have created a pull-request this soon. This should have been separated from the improvements in DB statement handling and done in a separate pull-request. It was suggested by others that I do the pull-request and got caught up in the whole process.

Just in case my documentation is unclear: When working from a VirtualBox instance of Ubuntu my manually created "~/.restore.conf" file contains these parameters:

DBHOST=localhost
DBUSER=magento
DBPASS=magpass
BASE_URL=http://192.168.56.131/

After that there is only the need to press 'enter' or 'return' to confirm the default or calculated values which are displayed in square brackets at each "read" statement.

reid@u14p55m56a:/var/www/03181220$ ./restore.sh
Enter DB host [localhost]: 
Enter DB name [03181220]: 
Enter DB user [magento]: 
Enter DB user's password [magpass]: 
Enter base url [http://192.168.56.131/]: 

Check parameters:
DB host is: localhost
DB name is: 03181220
DB user is: magento
DB pass is: magpass
Full base url is: http://192.168.56.131/03181220/
Continue? [YES/no]: no
Interrupted by user, exiting...
reid@u14p55m56a:/var/www/03181220$ 

Ultimately your use case is what I have in mind with options providing overrides to default values in the same style as the command line mysql client program and its corresponding "/.my.cnf" file. (CLI options would only be needed to override the "/.restore.conf" defaults.) This would probably require one extra 'return' to confirm the calculated settings (i.e. DB name based on working directory name). Also, I would like to add handling of table prefixes as we have only one schema to work with on Sparta.

I'd like to continue with this when I get the chance. Let me know.

@rwoodbury
Copy link
Copy Markdown
Author

Yeah, my docs weren't clear. I added some of my notes above to the README.

@rwoodbury
Copy link
Copy Markdown
Author

Here's an example of my planned handling of options:

Let's say my web root is "/var/www/" and my deployment is in "/var/www/9999/", and this one time I want to use a DB schema name different from the name matching the deployment directory ("9999") or override the name in the config file ("DBNAME"). The command would look like:

reid@u14p55m56a:~$ cd /var/www/9999/
reid@u14p55m56a:/var/www/9999$ ./restore.sh -d one_time_db_name

That's it. Everything else would come from the config file. The script would then output (using the sample config file previously mentioned):

Check parameters:
DB host is: localhost
DB name is: one_time_db_name
DB user is: magento
DB pass is: magpass
Full base url is: http://192.168.56.131/9999/
Continue? [YES/no]: no
Interrupted by user, exiting...
reid@u14p55m56a:/var/www/9999$ 

I'd probably leave out the "Continue? [YES/no]" confirmation and instead use a dry-run flag. Once a user has gotten used to how it works that one last confirmation will be one too many key strokes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants