Try refactor wp db query --defaults handling - #285
Closed
lkraav wants to merge 9 commits into
Closed
Conversation
Co-authored-by: lkraav <147228+lkraav@users.noreply.github.com>
- Replace empty foreach loop in get_current_sql_modes() with proper MySQL argument preservation using get_mysql_args() - Add ssl-verify-server-cert to allowed MySQL options list - Add test case to verify connection arguments are preserved during SQL mode discovery - Fixes issue where connection flags like --ssl-verify-server-cert were lost during SQL mode discovery Co-authored-by: lkraav <147228+lkraav@users.noreply.github.com>
github-actions
Bot
added
command:db-query
swissspidy
changed the title
wp db query --defaults handling, fix #237wp db query --defaults handling
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Member
|
Closed in favor of #333 |
Member
|
Thanks for the PR, @lkraav, even if we could not merge it. 🙏 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Try fix #237
Code change is all Copilot.
I reviewed it for general sanity, but I simply don't know enough
wp dbinternals to know whether this direction is where we want to go.But I'll file this anyway to get movement and directional discussion going.
I feel like currently
wp db queryjust doing--no-defaultsbasically hard-coded is overly naive for many real world use cases, and needs to be refactored no matter what. If you can't even connect to remote SSL-secured databases, I'm not sure how we can ignore this in 2025.My Copilot strategy session can be reviewed at lkraav#2
EDIT I've now also tested it, and this PR does make at least
--defaultsand/or--ssl-verify-server-certwork.