fix: add backwards compatibility for procedures #98
Conversation
DerekTBrown
changed the title
| user = "jdoe-%s" | ||
| host = "%s" | ||
| privileges = ["EXECUTE"] | ||
| database = "PROCEDURE %s.%s" |
There was a problem hiding this comment.
Maybe this is a too large of request, but could you please add test with the format that worked here before?
database = "PROCEDURE some_db_name"
table = "procedure_name"
I don't insist on this, but it would be helpful as a regression test.
There was a problem hiding this comment.
That's a good idea. I have introduced a test for this in the newest version.
There was a problem hiding this comment.
Thanks, that draft looks good - I have two more comments for you to comment (that were just resolved). Could you please at least comment on them?
| database = strings.Trim(m[3], "`\"") | ||
| table = strings.Trim(m[4], "`\"") | ||
| } else { | ||
| database = fmt.Sprintf("%s%s.%s", m[2], strings.Trim(m[3], "`\""), strings.Trim(m[4], "`\"")) |
There was a problem hiding this comment.
Please consider adding a comment the expected value. On a first sight, I had absolutely no idea about what should be saved to "database" and why.
There was a problem hiding this comment.
Did you do anything here? Please at least leave a comment if you want to resolve something.
There was a problem hiding this comment.
Sorry, I am actively working on this. Just haven't pushed my latest changes.
There was a problem hiding this comment.
Ok thanks! Feel free to push them; more changes can also be added later.
| @@ -23,7 +23,7 @@ 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 | |||
| PATH="$(CURDIR)/bin:${PATH}" TF_ACC=1 TF_LOG=DEBUG go test $(TEST) -v $(TESTARGS) -timeout=90s | |||
There was a problem hiding this comment.
Could you please remove this change?
It may be beneficial, but it fills test output with thousands of lines of TF debug that will be never read.
There was a problem hiding this comment.
Sorry, just haven't pushed changes.
What does this PR do?
This adds backwards-compatible support for procedures with the OSS terraform provider.
Test Plan