Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL-1310: Add release targets for ODBC on macos #131

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

bucaojit
Copy link
Member

@bucaojit bucaojit commented May 10, 2023

Added signing using the mac.sign evergreen task.

Requested credentials from the BUILD team for $key_id and $secret values. I will add them to our project and fully test the signing for this patch prior to merging.

Added the installer path to mongo-odbc-downloads_template.json.

Copy link
Member

@terakilobyte terakilobyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this produce files to test? I'm not seeing them in evergreen

@bucaojit
Copy link
Member Author

@terakilobyte I was actually expecting evergreen to fail since I don't have the credentials for the macnotary yet.
Turns out the macos/macos-arm buildvariants weren't in the project settings to run on patches. I have now added them. You're right, it will produce the *-signed dmg and dylib binaries once things are working.

@nbagnard
Copy link
Collaborator

Are we really comfortable releasing the mac driver without testing it with any tool? A sanity check with a tool supporting generic ODBC connections would be good, no?

@pmeredit
Copy link
Collaborator

I'm waiting on the credentials for this. Want to see it working.

@nbagnard we need to test with tableau imo

@bucaojit
Copy link
Member Author

Yes, that's a good idea to test with a tool supporting generic ODBC. I'm planning on testing this with Tableau when I get a new license key.

@bucaojit
Copy link
Member Author

I tried Tableau with the Intel driver and was able to connect. I used the DSN method and it showed the database that was specified in the DSN, not any tables or fields. I could manually input the table name, hit + and it finds it, but still doesn't show fields.

Looking at the trace, I see it has a problem with the SQLBindCol() command which is expected as it's not implemented:

tabprotosrv     204A64280 ENTER SQLBindCol
		SQLHSTMT          0x7fd7a4012580
		SQLUSMALLINT      1
		SQLSMALLINT       -8 (SQL_C_WCHAR)
		SQLPOINTER        0x7fd7a5030a00
		SQLLEN            1024
		SQLLEN          * 0x30cf846f8

[000033.072125]
tabprotosrv     204A64280 EXIT  SQLBindCol with return code -1 (SQL_ERROR)
		SQLHSTMT          0x7fd7a4012580
		SQLUSMALLINT      1
		SQLSMALLINT       -8 (SQL_C_WCHAR)
		SQLPOINTER        0x7fd7a5030a00
		SQLLEN            1024
		SQLLEN          * 0x30cf846f8

Checked with Natacha and we don't think this Mac driver is ready for release just yet.
I'll convert this PR into a draft and move SQ-1310 back into accepted.
@nbagnard I'm thinking I'll move this into another epic, SQL-580 looks like a good place for it?

@bucaojit bucaojit marked this pull request as draft May 15, 2023 20:57
@pmeredit
Copy link
Collaborator

I think it's probably fine that it's failing on that step since users will ideally use jdbc for tableau. We see that it's working other than that. We can just document that we do not yet support SQLBindCol, and probably file a ticket to support it (maybe an epic?)

@pmeredit
Copy link
Collaborator

I think it's probably fine that it's failing on that step since users will ideally use jdbc for tableau. We see that it's working other than that. We can just document that we do not yet support SQLBindCol, and probably file a ticket to support it (maybe an epic?)

Though, I do think it's best just to schedule the Mac support out as we discussed before. Ignore me here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants