Fix AWS authentication for Tableau on Mac #9
Conversation
-update code where necessary to address Mac warnings
src/elasticodbc/dlg_specific.h
Outdated
| @@ -52,6 +52,7 @@ extern "C" { | |||
| #endif /* UNICODE_SUPPORT */ | |||
|
|
|||
| #define INI_SERVER "host" | |||
| #define INI_SERVER_ALT "server" | |||
There was a problem hiding this comment.
In PR #7 I renamed this so that it's INI_HOST "host" and INI_SERVER "server"
There was a problem hiding this comment.
Merged in new changes
| @@ -67,7 +67,7 @@ int Ver1GEVer2(std::wstring ver_1_str, std::wstring ver_2_str) { | |||
| if ((ver_1.size() == 0) || (ver_2.size() == 0)) | |||
| return -1; | |||
|
|
|||
| size_t cnt = min(ver_1.size(), ver_2.size()); | |||
| size_t cnt = std::min(ver_1.size(), ver_2.size()); | |||
There was a problem hiding this comment.
As I worked on GitHub Actions I found that:
- with min(..) it worked for Windows, but not Mac
- with std::min(..) it worked for Mac, but not for Windows.
Which is the annoying result of the std and min macro colliding in C / C++...
I used ((ver_1.size() < ver_2.size()) ? ver_1.size() : ver_2.size()); in place of this here to get rid of that issue.
There was a problem hiding this comment.
applied suggestion
| static const std::string ALLOCATION_TAG = "AWS_SIGV4_AUTH"; | ||
| static const std::string SERVICE_NAME = "es"; | ||
| static const std::string ESODBC_PROFILE_NAME = "elasticsearchodbc"; |
There was a problem hiding this comment.
Output file of build / driver project name is elasticodbc. Do we want this to be elasticsearchodbc or just elasticodbc?
There was a problem hiding this comment.
This name will be checked against available profiles in ./aws/credentials file so it should be fine
# Conflicts: # src/elasticodbc/dlg_specific.c # src/elasticodbc/dlg_specific.h # src/elasticodbc/es_communication.cpp
|
Confirmed that these changes work with Tableau for Mac (ie. can create a chart using data from an AWS-authenticated Elasticsearch server) |
# Conflicts: # src/IntegrationTests/ITODBCDescriptors/test_odbc_descriptors.cpp # src/UnitTests/UTConn/test_conn.cpp # src/UnitTests/UTConn/test_query_execution.cpp # src/elasticodbc/CMakeLists.txt
Issue #, if available: #8
Description of changes:
This enables the use of AWS_SIGV4 authentication using the driver with Tableau on Mac.
Current behaviour tested:
Pending testing:
Note: Currently waiting on the SQL plugin to be updated on the AWS server that has been provided for us, which prevents full testing with Tableau.
Note: Changes still need to be tested on Windows
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.