Only unset defaults before running command. #279
Conversation
… in querying the SQL mode
This comment was marked as resolved.
This comment was marked as resolved.
swissspidy
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the handling of the --defaults flag by centralizing its unsetting to the run method. This ensures that the flag is correctly processed by get_defaults_flag_string to determine whether --no-defaults should be added to the command, and then removed before being passed to the underlying MySQL command execution. This improves the maintainability and correctness of argument handling, especially for commands that query SQL modes, as the defaults flag is now available for evaluation until the final command execution step. The changes are well-implemented and align with the stated objective.
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where database queries fail when using TLS/SSL connections with non-standard certificate authorities. The issue occurred because the --defaults flag was being removed too early in the execution flow, preventing MySQL client configuration files (which may contain TLS settings) from being loaded when querying SQL modes.
Changes:
- Move the removal of the
defaultskey fromget_defaults_flag_string()to therun()method, allowing MySQL defaults files to be used when querying SQL modes while still preventing the invalid argument from being passed to MySQL
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ( true === Utils\get_flag_value( $assoc_args, 'defaults' ) ) { | ||
| $flag_string = ''; | ||
| } | ||
|
|
||
| unset( $assoc_args['defaults'] ); | ||
|
|
||
| } |
There was a problem hiding this comment.
Since get_defaults_flag_string() no longer removes the 'defaults' key from the array, consider updating the method signature from protected function get_defaults_flag_string( &$assoc_args ) to protected function get_defaults_flag_string( $assoc_args ) to remove the unnecessary by-reference parameter.
4 participants
Allow defaults to be used in querying the SQL mode
Fixes #278