Fixes TF drift for resource group name and query_limit #6
Conversation
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
otherchen
commented
Aug 9, 2024
| @@ -34,8 +34,7 @@ func (rg *ResourceGroup) buildSQLQuery(prefix string) string { | |||
| query = append(query, fmt.Sprintf(`PRIORITY = %s`, rg.Priority)) | |||
|
|
|||
| if rg.QueryLimit != DefaultResourceGroup.QueryLimit { | |||
| query = append(query, fmt.Sprintf(`QUERY_LIMIT=%s`, rg.QueryLimit)) | |||
|
|
|||
| query = append(query, fmt.Sprintf(`QUERY_LIMIT=(%s)`, rg.QueryLimit)) | |||
There was a problem hiding this comment.
We rely on wrapping the query_limit in parentheses here instead of assuming that it will be wrapped in parentheses by the caller.
otherchen
commented
Aug 9, 2024
| @@ -126,7 +126,7 @@ func DeleteResourceGroupUser(ctx context.Context, d *schema.ResourceData, meta i | |||
| } | |||
|
|
|||
| func readUserFromDB(db *sql.DB, name string) (string, string, error) { | |||
| selectUsersQuery := `SELECT USER, IFNULL(JSON_EXTRACT(User_attributes, "$.resource_group"), "") as resource_group FROM mysql.user WHERE USER = ?` | |||
| selectUsersQuery := `SELECT USER, JSON_UNQUOTE(IFNULL(JSON_EXTRACT(User_attributes, "$.resource_group"), "")) as resource_group FROM mysql.user WHERE USER = ?` | |||
There was a problem hiding this comment.
This gets rid of the quotes that JSON_EXTRACT will generate.
zph
reviewed
Aug 9, 2024
GNUmakefile
Outdated
| @@ -43,7 +43,12 @@ bin/terraform: | |||
| (cd $(CURDIR)/bin/ ; unzip terraform.zip) | |||
|
|
|||
| testacc: fmtcheck bin/terraform | |||
| PATH="$(CURDIR)/bin:${PATH}" TF_ACC=1 go test $(TEST) -v $(TESTARGS) -timeout=90s | |||
There was a problem hiding this comment.
This will be a potentially breaking change to upstream
zph
reviewed
Aug 9, 2024
GNUmakefile
Outdated
| # Commenting out the below because it tries to use an amd64 version of terraform that is incompatible with M1 Macs. Instead, | ||
| # simply rely on the version of terraform that's already on your path. If you don't have terraform installed, follow instructions | ||
| # here to install https://developer.hashicorp.com/terraform/tutorials/aws-get-started/install-cli | ||
| # PATH="$(CURDIR)/bin:${PATH}" TF_ACC=1 go test $(TEST) -v $(TESTARGS) -run "TestTIDBResourceGroup_basic" -timeout=90s |
There was a problem hiding this comment.
Do we need this as a separate section in Makefile or can we remove it since I've patched the ARCH problem?
There was a problem hiding this comment.
I'm removing it since it's patched for ARCH above.
LMK if you disagree
There was a problem hiding this comment.
Sounds good! Thanks for fixing this 👍
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.
This fixes the following drift:
()parentheses when pulled from the DB. This PR standardizes around the query_limit inside the parentheses. A breaking change is that we expect NO parenthesis to be passed in for the query_limit from now on. Instead, we will wrap the query_limit inside parentheses in the actual query rather than rely on the query_limit string being wrapped in parentheses by the caller.I also got basic tidb tests working and tested the above changes using
make testtidb7.5.1