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

[database:dump] display sql error #4152

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lalop
Copy link
Contributor

@lalop lalop commented Sep 16, 2019

No description provided.

@enzolutions
Copy link
Contributor

@lalop thanks for your contribution, could you create an issue where you could provide the case, of why is a good idea to remove the try and what we will accomplist with the change that you propose in this PR.

@lalop
Copy link
Contributor Author

lalop commented Sep 18, 2019

I tried to start the discussion on slack. Since the try/catch is used to hide a usefull error message I don't see why it should stay here

@lalop
Copy link
Contributor Author

lalop commented Sep 24, 2019

Here is the restult whithout try/catch

$ drupal database:dump 

In Process.php line 239:
                                                                                                                                                                                    
  The command "mysqldump --user='root' --password='rootpass' --host='drupaldb' --port='3306' 'lido' > '/Users/sam/Workspace/projets/Puppets/lido/site/web/lido-2019-09-24-11-40-51  
  .sql'" failed.                                                                                                                                                                    
                                                                                                                                                                                    
  Exit Code: 127(Command not found)                                                                                                                                                 
                                                                                                                                                                                    
  Working directory: /Users/sam/Workspace/projets/Puppets/lido/site/web                                                                                                             
                                                                                                                                                                                    
  Output:                                                                                                                                                                           
  ================                                                                                                                                                                  
                                                                                                                                                                                    
                                                                                                                                                                                    
  Error Output:                                                                                                                                                                     
  ================                                                                                                                                                                  
  sh: mysqldump: command not found                                                                                                                                                  
                                                                                                                                                                                    

database:dump [--file [FILE]] [--gz] [--exclude-cache] [-h|--help] [-q|--quiet] [-v|vv|vvv|--verbose] [-V|--version] [--ansi] [--no-ansi] [-n|--no-interaction] [-e|--env [ENV]] [--root [ROOT]] [--debug] [--learning] [-c|--generate-chain] [-i|--generate-inline] [-d|--generate-doc] [-t|--target [TARGET]] [-l|--uri URI] [-y|--yes] [--] <command> [<database>] [<target>]

here is with $this->getIo()->error

$ drupal database:dump 

                                                                                                                        
 [ERROR] commands.database.dump.messages.error                                                                          

@enzolutions
Copy link
Contributor

@lalop so you said there is no form to present the following error with a better format using try-catch?

  Error Output:                                                                                                                                                                     
  ================                                                                                                                                                                  
  sh: mysqldump: command not found 

@lalop
Copy link
Contributor Author

lalop commented Sep 25, 2019

Not really, here is what I said on slack

the try/catch is done by symfony https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Console/Application.php#L145 so adding a try and catch on each command is a lot of repeated code for a small profit in my point of view

@lalop
Copy link
Contributor Author

lalop commented Oct 23, 2019

@enzolutions what is the status of this pr ?

@enzolutions
Copy link
Contributor

@lalop I didn't have time yet to review in details, I will try to do next week

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