test(csharp/test/Drivers/Snowflake): refactor snowflake test framework #1323
Conversation
|
|
ruowan
changed the title
|
@davidhcoe Help to review this PR. |
lidavidm
changed the title
There was a problem hiding this comment.
There are a number of changes that need to be addressed before this can be checked in. It makes several breaking changes to the Snowflake DriverTests. I attempted to check in some changes but don't have the right permissions, so I tried to add them individually. Please ensure all the tests pass (when successful, there are two that still fail, but those are addressed in other PRs/issues).
Depending on when this lands, it may be impacted by some of the refactoring in https://github.com/apache/arrow-adbc/pull/1328/files.
| } | ||
| } | ||
| else | ||
| { | ||
| builder[SnowflakeParameters.PASSWORD] = testConfiguration.Password; | ||
| // default basic auth |
There was a problem hiding this comment.
This needs to specify auth_snowflake (what is currently called Default)
| "private_key_pwd": "", | ||
| "user": "" | ||
| }, | ||
| "default": { |
There was a problem hiding this comment.
"default" is not an auth type. This should be auth_snowflake
|
I run the test cases all test case succeeded expect the two failure test cases you mentioned before. |
|
It's failing some of the tests related to @ryan-syed's PR but everything seems OK other than the one comment about specifying Were you also going to look at defining auth in a way that could be used with different queries? |
|
I am not looking at support different query, yet. But, That's a good point that we can enhance testFramework to support different query. I will log it as a work item. |
|
@ruowan - can you resolve the merge conflicts and then this should be good to go. |
|
Resolved. I think we can merge this PR. |
No description provided.